Re: [PATCH v3 3/4] KVM: x86: Add EOI exit bitmap inference
On 09/06/2015 04:16, Wanpeng Li wrote: >> >> So in the end the patched vcpu_scan_ioapic becomes >> >> if (kvm_apic_hw_enabled(vcpu->arch.apic)) > > s/kvm_apic_hw_enabled(vcpu->arch.apic)/!kvm_apic_hw_enabled(vcpu->arch.apic) Right, thanks for the correction. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] KVM: x86: Add EOI exit bitmap inference
On 6/8/15 10:15 PM, Paolo Bonzini wrote:
On 08/06/2015 12:33, Wanpeng Li wrote:
+if (kvm_check_request(KVM_REQ_SCAN_IOAPIC, vcpu)) {
+if (irqchip_split(vcpu->kvm)) {
+memset(vcpu->arch.eoi_exit_bitmaps, 0, 32);
+kvm_scan_ioapic_routes(
+vcpu, vcpu->arch.eoi_exit_bitmaps);
+kvm_x86_ops->load_eoi_exitmap(
+vcpu, vcpu->arch.eoi_exit_bitmaps);
How about introduce a handler to fold above just like vcpu_scan_ioapic() ?
Or just move the "if" inside the existing vcpu_scan_ioapic(). This is a
good suggestion because it raises more question:
1) should the
if (!kvm_apic_hw_enabled(vcpu->arch.apic))
return;
of vcpu_scan_ioapic apply to the "split irqchip" case too?
2) the "non-split irqchip" can also use vcpu->arch.eoi_exit_bitmaps
instead of the local eoi_exit_bitmap variable of vcpu_scan_ioapic.
So in the end the patched vcpu_scan_ioapic becomes
if (kvm_apic_hw_enabled(vcpu->arch.apic))
s/kvm_apic_hw_enabled(vcpu->arch.apic)/!kvm_apic_hw_enabled(vcpu->arch.apic)
Regards,
Wanpeng Li
return;
memset(vcpu->arch.eoi_exit_bitmaps, 0, 32);
if (irqchip_split)
scan_ioapic_routes(...)
else {
memset(tmr, 0, 32)
kvm_ioapic_scan_entry(...)
kvm_apic_update_tmr(vcpu, tmr);
}
kvm_x86_ops->load_eoi_exitmap(vcpu,
vcpu->arch.eoi_exit_bitmaps);
A FIXME/TODO comment about TMR in the split-irqchip case is probably in
order too.
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] KVM: x86: Add EOI exit bitmap inference
On 08/06/2015 12:33, Wanpeng Li wrote:
>> +if (kvm_check_request(KVM_REQ_SCAN_IOAPIC, vcpu)) {
>> +if (irqchip_split(vcpu->kvm)) {
>> +memset(vcpu->arch.eoi_exit_bitmaps, 0, 32);
>> +kvm_scan_ioapic_routes(
>> +vcpu, vcpu->arch.eoi_exit_bitmaps);
>> +kvm_x86_ops->load_eoi_exitmap(
>> +vcpu, vcpu->arch.eoi_exit_bitmaps);
>
> How about introduce a handler to fold above just like vcpu_scan_ioapic() ?
Or just move the "if" inside the existing vcpu_scan_ioapic(). This is a
good suggestion because it raises more question:
1) should the
if (!kvm_apic_hw_enabled(vcpu->arch.apic))
return;
of vcpu_scan_ioapic apply to the "split irqchip" case too?
2) the "non-split irqchip" can also use vcpu->arch.eoi_exit_bitmaps
instead of the local eoi_exit_bitmap variable of vcpu_scan_ioapic.
So in the end the patched vcpu_scan_ioapic becomes
if (kvm_apic_hw_enabled(vcpu->arch.apic))
return;
memset(vcpu->arch.eoi_exit_bitmaps, 0, 32);
if (irqchip_split)
scan_ioapic_routes(...)
else {
memset(tmr, 0, 32)
kvm_ioapic_scan_entry(...)
kvm_apic_update_tmr(vcpu, tmr);
}
kvm_x86_ops->load_eoi_exitmap(vcpu,
vcpu->arch.eoi_exit_bitmaps);
A FIXME/TODO comment about TMR in the split-irqchip case is probably in
order too.
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] KVM: x86: Add EOI exit bitmap inference
On 6/3/15 7:51 AM, Steve Rutherford wrote:
In order to support a userspace IOAPIC interacting with an in kernel
APIC, the EOI exit bitmaps need to be configurable.
If the IOAPIC is in userspace (i.e. the irqchip has been split), the
EOI exit bitmaps will be set whenever the GSI Routes are configured.
In particular, for the low MSI routes are reservable for userspace
IOAPICs. For these MSI routes, the EOI Exit bit corresponding to the
destination vector of the route will be set for the destination VCPU.
The intention is for the userspace IOAPICs to use the reservable MSI
routes to inject interrupts into the guest.
This is a slight abuse of the notion of an MSI Route, given that MSIs
classically bypass the IOAPIC. It might be worthwhile to add an
additional route type to improve clarity.
Compile tested for Intel x86.
Signed-off-by: Steve Rutherford
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/ioapic.c | 16
arch/x86/kvm/ioapic.h | 2 ++
arch/x86/kvm/lapic.c| 3 +--
arch/x86/kvm/x86.c | 30 ++
include/linux/kvm_host.h| 9 +
virt/kvm/irqchip.c | 37 +
7 files changed, 88 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2778d36..4f439ff 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -644,6 +644,7 @@ struct kvm_arch {
u64 disabled_quirks;
bool irqchip_split;
+ u8 nr_reserved_ioapic_pins;
};
struct kvm_vm_stat {
diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 856f791..fb5281b 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -672,3 +672,19 @@ int kvm_set_ioapic(struct kvm *kvm, struct
kvm_ioapic_state *state)
spin_unlock(&ioapic->lock);
return 0;
}
+
+void kvm_arch_irq_routing_update(struct kvm *kvm)
+{
+ struct kvm_ioapic *ioapic = kvm->arch.vioapic;
+
+ if (ioapic)
+ return;
+ if (!lapic_in_kernel(kvm))
+ return;
+ kvm_make_scan_ioapic_request(kvm);
+}
+
+u8 kvm_arch_nr_userspace_ioapic_pins(struct kvm *kvm)
+{
+ return kvm->arch.nr_reserved_ioapic_pins;
+}
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index ca0b0b4..3af349c 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -9,6 +9,7 @@ struct kvm;
struct kvm_vcpu;
#define IOAPIC_NUM_PINS KVM_IOAPIC_NUM_PINS
+#define MAX_NR_RESERVED_IOAPIC_PINS 48
#define IOAPIC_VERSION_ID 0x11/* IOAPIC version */
#define IOAPIC_EDGE_TRIG 0
#define IOAPIC_LEVEL_TRIG 1
@@ -123,4 +124,5 @@ int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state
*state);
void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
u32 *tmr);
+void kvm_scan_ioapic_routes(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
#endif
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 28eb946..766d297 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -209,8 +209,7 @@ out:
if (old)
kfree_rcu(old, rcu);
- if (!irqchip_split(kvm))
- kvm_vcpu_request_scan_ioapic(kvm);
+ kvm_make_scan_ioapic_request(kvm);
}
static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5e01810..35d13d4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3930,15 +3930,20 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
case KVM_CAP_SPLIT_IRQCHIP: {
mutex_lock(&kvm->lock);
r = -EEXIST;
- if (lapic_in_kernel(kvm))
+ if (irqchip_in_kernel(kvm))
goto split_irqchip_unlock;
r = -EINVAL;
- if (atomic_read(&kvm->online_vcpus))
- goto split_irqchip_unlock;
- r = kvm_setup_empty_irq_routing(kvm);
- if (r)
+ if (cap->args[0] > MAX_NR_RESERVED_IOAPIC_PINS)
goto split_irqchip_unlock;
- kvm->arch.irqchip_split = true;
+ if (!irqchip_split(kvm)) {
+ if (atomic_read(&kvm->online_vcpus))
+ goto split_irqchip_unlock;
+ r = kvm_setup_empty_irq_routing(kvm);
+ if (r)
+ goto split_irqchip_unlock;
+ kvm->arch.irqchip_split = true;
+ }
+ kvm->arch.nr_reserved_ioapic_pins = cap->args[0];
r = 0;
split_irqchip_unlock:
mutex_unlock(&kvm->lock);
@@ -6403,8 +6408,17 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
goto out;
}
}
- if (kvm_check_request(
Re: [PATCH v3 3/4] KVM: x86: Add EOI exit bitmap inference
On Wed, Jun 03, 2015 at 11:16:16AM +0200, Paolo Bonzini wrote:
>
>
> On 03/06/2015 01:51, Steve Rutherford wrote:
> > +static inline void kvm_arch_irq_routing_update(struct kvm *kvm)
> > +{
> > +}
>
> Please add the static inline to all arches instead of putting it in
> #ifndef __KVM_HAVE_IOAPIC. It's not related to the existence of an ioapic.
>
> >
> > +void kvm_arch_irq_routing_update(struct kvm *kvm)
> > +{
> > + struct kvm_ioapic *ioapic = kvm->arch.vioapic;
> > +
> > + if (ioapic)
> > + return;
> > + if (!lapic_in_kernel(kvm))
> > + return;
> > + kvm_make_scan_ioapic_request(kvm);
> > +}
> > +
>
> It's weird to have a function in ioapic.c that only does something if
> you _do not_ have an ioapic. :)
>
> > +
> > +void kvm_scan_ioapic_routes(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
>
> This must stay in arch/x86/kvm/. I'd put both of these in irq_comm.c.
>
> Then you do not need kvm_arch_nr_userspace_ioapic_pins anymore.
Yes! This is way cleaner. I'll make these changes.
Steve
>
> Paolo
>
> > +{
> > + struct kvm *kvm = vcpu->kvm;
> > + struct kvm_kernel_irq_routing_entry *entry;
> > + struct kvm_irq_routing_table *table;
> > + u32 i, nr_ioapic_pins;
> > + int idx;
> > +
> > + /* kvm->irq_routing must be read after clearing
> > +* KVM_SCAN_IOAPIC. */
> > + smp_mb();
> > + idx = srcu_read_lock(&kvm->irq_srcu);
> > + table = kvm->irq_routing;
> > + nr_ioapic_pins = min_t(u32, table->nr_rt_entries,
> > + kvm_arch_nr_userspace_ioapic_pins(kvm));
> > + for (i = 0; i < nr_ioapic_pins; ++i) {
> > + hlist_for_each_entry(entry, &table->map[i], link) {
> > + u32 dest_id, dest_mode;
> > +
> > + if (entry->type != KVM_IRQ_ROUTING_MSI)
> > + continue;
> > + dest_id = (entry->msi.address_lo >> 12) & 0xff;
> > + dest_mode = (entry->msi.address_lo >> 2) & 0x1;
> > + if (kvm_apic_match_dest(vcpu, NULL, 0, dest_id,
> > + dest_mode)) {
> > + u32 vector = entry->msi.data & 0xff;
> > +
> > + __set_bit(vector,
> > + (unsigned long *) eoi_exit_bitmap);
> > + }
> > + }
> > + }
> > + srcu_read_unlock(&kvm->irq_srcu, idx);
> > +}
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] KVM: x86: Add EOI exit bitmap inference
On 03/06/2015 01:51, Steve Rutherford wrote:
> +static inline void kvm_arch_irq_routing_update(struct kvm *kvm)
> +{
> +}
Please add the static inline to all arches instead of putting it in
#ifndef __KVM_HAVE_IOAPIC. It's not related to the existence of an ioapic.
>
> +void kvm_arch_irq_routing_update(struct kvm *kvm)
> +{
> + struct kvm_ioapic *ioapic = kvm->arch.vioapic;
> +
> + if (ioapic)
> + return;
> + if (!lapic_in_kernel(kvm))
> + return;
> + kvm_make_scan_ioapic_request(kvm);
> +}
> +
It's weird to have a function in ioapic.c that only does something if
you _do not_ have an ioapic. :)
> +
> +void kvm_scan_ioapic_routes(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
This must stay in arch/x86/kvm/. I'd put both of these in irq_comm.c.
Then you do not need kvm_arch_nr_userspace_ioapic_pins anymore.
Paolo
> +{
> + struct kvm *kvm = vcpu->kvm;
> + struct kvm_kernel_irq_routing_entry *entry;
> + struct kvm_irq_routing_table *table;
> + u32 i, nr_ioapic_pins;
> + int idx;
> +
> + /* kvm->irq_routing must be read after clearing
> + * KVM_SCAN_IOAPIC. */
> + smp_mb();
> + idx = srcu_read_lock(&kvm->irq_srcu);
> + table = kvm->irq_routing;
> + nr_ioapic_pins = min_t(u32, table->nr_rt_entries,
> + kvm_arch_nr_userspace_ioapic_pins(kvm));
> + for (i = 0; i < nr_ioapic_pins; ++i) {
> + hlist_for_each_entry(entry, &table->map[i], link) {
> + u32 dest_id, dest_mode;
> +
> + if (entry->type != KVM_IRQ_ROUTING_MSI)
> + continue;
> + dest_id = (entry->msi.address_lo >> 12) & 0xff;
> + dest_mode = (entry->msi.address_lo >> 2) & 0x1;
> + if (kvm_apic_match_dest(vcpu, NULL, 0, dest_id,
> + dest_mode)) {
> + u32 vector = entry->msi.data & 0xff;
> +
> + __set_bit(vector,
> + (unsigned long *) eoi_exit_bitmap);
> + }
> + }
> + }
> + srcu_read_unlock(&kvm->irq_srcu, idx);
> +}
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/4] KVM: x86: Add EOI exit bitmap inference
In order to support a userspace IOAPIC interacting with an in kernel
APIC, the EOI exit bitmaps need to be configurable.
If the IOAPIC is in userspace (i.e. the irqchip has been split), the
EOI exit bitmaps will be set whenever the GSI Routes are configured.
In particular, for the low MSI routes are reservable for userspace
IOAPICs. For these MSI routes, the EOI Exit bit corresponding to the
destination vector of the route will be set for the destination VCPU.
The intention is for the userspace IOAPICs to use the reservable MSI
routes to inject interrupts into the guest.
This is a slight abuse of the notion of an MSI Route, given that MSIs
classically bypass the IOAPIC. It might be worthwhile to add an
additional route type to improve clarity.
Compile tested for Intel x86.
Signed-off-by: Steve Rutherford
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/ioapic.c | 16
arch/x86/kvm/ioapic.h | 2 ++
arch/x86/kvm/lapic.c| 3 +--
arch/x86/kvm/x86.c | 30 ++
include/linux/kvm_host.h| 9 +
virt/kvm/irqchip.c | 37 +
7 files changed, 88 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2778d36..4f439ff 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -644,6 +644,7 @@ struct kvm_arch {
u64 disabled_quirks;
bool irqchip_split;
+ u8 nr_reserved_ioapic_pins;
};
struct kvm_vm_stat {
diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 856f791..fb5281b 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -672,3 +672,19 @@ int kvm_set_ioapic(struct kvm *kvm, struct
kvm_ioapic_state *state)
spin_unlock(&ioapic->lock);
return 0;
}
+
+void kvm_arch_irq_routing_update(struct kvm *kvm)
+{
+ struct kvm_ioapic *ioapic = kvm->arch.vioapic;
+
+ if (ioapic)
+ return;
+ if (!lapic_in_kernel(kvm))
+ return;
+ kvm_make_scan_ioapic_request(kvm);
+}
+
+u8 kvm_arch_nr_userspace_ioapic_pins(struct kvm *kvm)
+{
+ return kvm->arch.nr_reserved_ioapic_pins;
+}
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index ca0b0b4..3af349c 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -9,6 +9,7 @@ struct kvm;
struct kvm_vcpu;
#define IOAPIC_NUM_PINS KVM_IOAPIC_NUM_PINS
+#define MAX_NR_RESERVED_IOAPIC_PINS 48
#define IOAPIC_VERSION_ID 0x11 /* IOAPIC version */
#define IOAPIC_EDGE_TRIG 0
#define IOAPIC_LEVEL_TRIG 1
@@ -123,4 +124,5 @@ int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state
*state);
void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
u32 *tmr);
+void kvm_scan_ioapic_routes(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
#endif
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 28eb946..766d297 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -209,8 +209,7 @@ out:
if (old)
kfree_rcu(old, rcu);
- if (!irqchip_split(kvm))
- kvm_vcpu_request_scan_ioapic(kvm);
+ kvm_make_scan_ioapic_request(kvm);
}
static inline void apic_set_spiv(struct kvm_lapic *apic, u32 val)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5e01810..35d13d4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3930,15 +3930,20 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
case KVM_CAP_SPLIT_IRQCHIP: {
mutex_lock(&kvm->lock);
r = -EEXIST;
- if (lapic_in_kernel(kvm))
+ if (irqchip_in_kernel(kvm))
goto split_irqchip_unlock;
r = -EINVAL;
- if (atomic_read(&kvm->online_vcpus))
- goto split_irqchip_unlock;
- r = kvm_setup_empty_irq_routing(kvm);
- if (r)
+ if (cap->args[0] > MAX_NR_RESERVED_IOAPIC_PINS)
goto split_irqchip_unlock;
- kvm->arch.irqchip_split = true;
+ if (!irqchip_split(kvm)) {
+ if (atomic_read(&kvm->online_vcpus))
+ goto split_irqchip_unlock;
+ r = kvm_setup_empty_irq_routing(kvm);
+ if (r)
+ goto split_irqchip_unlock;
+ kvm->arch.irqchip_split = true;
+ }
+ kvm->arch.nr_reserved_ioapic_pins = cap->args[0];
r = 0;
split_irqchip_unlock:
mutex_unlock(&kvm->lock);
@@ -6403,8 +6408,17 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
goto out;
}
}
- if (kvm_check_request(KVM_REQ_SCAN_IOAPIC, vcpu))
- vcpu_scan_ioapic(vcpu);
