[Qemu-devel] [RFC PATCH v2] kvm: x86: ignore ioapic polarity

["Gabriel L. Somlo" <gsomlo@gmail.com>]

Both QEMU and KVM have already accumulated a significant number of
optimizations based on the hard-coded assumption that ioapic polarity
will always use the ActiveHigh convention, where the logical and
physical states of level-triggered irq lines always match (i.e.,
active(asserted) == high == 1, inactive == low == 0). QEMU guests
are expected to follow directions given via ACPI and configure the
ioapic with polarity 0 (ActiveHigh). However, even when misbehaving
guests (e.g. OS X <= 10.9) set the ioapic polarity to 1 (ActiveLow),
QEMU will still use the ActiveHigh signaling convention when
interfacing with KVM.

This patch modifies KVM to completely ignore ioapic polarity as set by
the guest OS, enabling misbehaving guests to work alongside those which
comply with the ActiveHigh polarity specified by QEMU's ACPI tables.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Gabriel L. Somlo <somlo@cmu.edu>
---

On Fri, Feb 28, 2014 at 12:31:27AM +0100, Paolo Bonzini wrote:
>>>This is a much better description.  Can you turn it into a patch to
>>>Documentation/virtual/kvm/api.txt and a more complete commit
>>>message?

OK, let me know what you all think of this version.

> If you change ACPI tables to ActiveLow, and leave the polarity
> inversion in hw/intc/ioapic.c, then it will matter.

I only changed ACPI to ActiveLow to cause Linux to misbehave in the
same way OS X does, it was a temporary/test hack. ACPI should stay
ActiveHigh to reflect the rest of QEMU/KVM, and misbehaving guest OSs
should ignore it at their own peril :)

> >(Hmmm, maybe this one of the reasons I never got OS X to boot without
> >-enable-kvm... I should look at the QEMU hw/intc/ioapic*.c, and see
> >if *it* cares about guest-configured polarity, and maybe get it to
> >*stop* caring :)

> Exactly my point. :)

So this patch gets the accelerated path to work regardless of how
well-behaved a guest OS may or may not be w.r.t. ACPI and polarity.
I'll try to find out whether it's possible to be *extra* nice to these
types of guests by also allowing them to ignore ACPI polarity when not
using acceleration :)

Thanks again,
  Gabriel

 Documentation/virtual/kvm/api.txt | 18 ++++++++++++++++++
 include/uapi/linux/kvm.h          |  1 +
 virt/kvm/ioapic.c                 |  1 -
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 6cd63a9..0492a94 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1365,6 +1365,24 @@ struct kvm_irq_routing_msi {
 	__u32 pad;
 };
 
+NOTE: For each level-triggered interrupt managed by a virtual ioapic,
+the guest OS may set a polarity value (bit 13 of each corresponding I/O
+redirection table register). The polarity bit defines the relationship
+between an irq line's logical state (active/asserted = 1, inactive = 0)
+and its physical state (high = 1, low = 0). When the polarity bit is 0
+(ActiveHigh), logical and physical states are matched (active == high == 1,
+inactive == low == 0). When polarity is set to 1 (ActiveLow), logical and
+physical state are opposites (logical == !physical). Typically, guests are
+expected to use the same polarity across all level-triggered interrupts,
+as directed by ACPI. Historically, both KVM and QEMU have accumulated a
+significant number of optimizations based on the hard-coded assumption
+that polarity will always be set to ActiveHigh. When interfacing with KVM,
+QEMU will use the ActiveHigh convention for all level-triggered irqs,
+regardless of how the guest OS has configured the polarity bits in the
+ioapic registers. As such, KVM must also ignore these bits, and always act
+as if the logical and physical states of an irq line exactly match each
+other (i.e., follow the ActiveHigh convention regardless of polarity).
+
 
 4.53 KVM_ASSIGN_SET_MSIX_NR
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 932d7f2..5bfc0e6 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -675,6 +675,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_SPAPR_MULTITCE 94
 #define KVM_CAP_EXT_EMUL_CPUID 95
 #define KVM_CAP_HYPERV_TIME 96
+#define KVM_CAP_X86_IOAPIC_POLARITY_IGNORED 97
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index ce9ed99..1539d37 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -328,7 +328,6 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
 	irq_level = __kvm_irq_line_state(&ioapic->irq_states[irq],
 					 irq_source_id, level);
 	entry = ioapic->redirtbl[irq];
-	irq_level ^= entry.fields.polarity;
 	if (!irq_level) {
 		ioapic->irr &= ~mask;
 		ret = 1;
-- 
1.8.1.4