diff mbox

[RFC] kvm: ignore apic polarity

Message ID 20140227170549.GA23037@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Feb. 27, 2014, 5:05 p.m. UTC
apic polarity in KVM does not work: too many things assume active high.
Let's not pretend it works, let's just ignore polarity flag.  If we ever
want to emulate it exactly, this will need a feature flag anyway.

Also report this to userspace: this makes it
possible to report the interrupt active-low
in ACPI, this way we are closer to real hardware.

This patch fixes OSX running on KVM.

Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

Gabriel, could you confirm this fixes OSX for you?
If you can play with linux tweaking interrupt
to active low, that would be very nice too:
it's weekend here.

Comments

Gabriel L. Somlo Feb. 27, 2014, 9:41 p.m. UTC | #1
On Thu, Feb 27, 2014 at 07:05:49PM +0200, Michael S. Tsirkin wrote:
> apic polarity in KVM does not work: too many things assume active high.
> Let's not pretend it works, let's just ignore polarity flag.  If we ever
> want to emulate it exactly, this will need a feature flag anyway.
> 
> Also report this to userspace: this makes it
> possible to report the interrupt active-low
> in ACPI, this way we are closer to real hardware.
> 
> This patch fixes OSX running on KVM.
> 
> Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> 
> Gabriel, could you confirm this fixes OSX for you?
> If you can play with linux tweaking interrupt
> to active low, that would be very nice too:
> it's weekend here.

With Fedora 20 Live x86_64:

If all I do is 's/ActiveHigh/ActiveLow/' in hw/i386/[q36-]acpi-dsdt.dsl,
but otherwise don't try to change how QEMU deals with "logical" vs.
"physical" ioapic polarity, things work great. Printk's show polarity
set to 1, but with the ignore-polarity patch things work fine.

With normal (ActiveHigh) ACPI, printk reports polarity set to 0, and
things *still* work exactly the same.


So, the way I understand this (and I'm writing this mainly for myself,
to make sure I understand correctly, so please kick me if I got it
wrong), ACPI tells the guest OS how to configure "physical" ioapic polarity.

With ActiveHigh, "physical" == "logical", i.e. "high" == "asserted"
and "low" == "deasserted".

With ActiveLow, "physical" == !"logical", so the other way around.

QEMU being hard-coded to ActiveHigh is the moral equivalent of always
sending the kernel (KVM) "logical" line states, rather than "physical"
ones.

Assuming KVM's userland clients are all coded for ActiveHigh, we can
(should, for sanity's sake) just assume line states from userland are
logical, and stop paying attention the polarity bits. That way,
misbehaving guests [*] can configure their ioapics as they please, and
things will just work OK regardless.

As you pointed out earlier, even KVM itself already kind-of assumes
ActiveHigh (e.g. in __kvm_irq_line_state(), which should be coded
differently if ActiveLow were a serious possibility, and, BTW,
irq_states[irq] would probably have to be initialized to all-1's if
ActiveLow wre used, etc, etc).


[*] All Apple hardware is ActiveLow, and OS X has this as a hardcoded
assumption (it ignores ACPI's hints to the contrary). So OS X is one
of the misbehaving clients I mention above...


> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 902f124..db29b27 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -674,6 +676,7 @@ struct kvm_ppc_smmu_info {
>  #define KVM_CAP_ARM_EL1_32BIT 93
>  #define KVM_CAP_SPAPR_MULTITCE 94
>  #define KVM_CAP_EXT_EMUL_CPUID 95
> +#define KVM_CAP_X86_IOAPIC_POLARITY_IGNORED 96

For one thing, on current kvm git, this conflicts with

#define KVM_CAP_HYPERV_TIME 96

but the rest of the patch works (I've already been using it for quite
a while to get OS X guests to work...

Thanks much,
--Gabriel

>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 2d68297..ad170b4 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;
> --
> 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
Paolo Bonzini Feb. 27, 2014, 10:30 p.m. UTC | #2
Il 27/02/2014 22:41, Gabriel L. Somlo ha scritto:
> On Thu, Feb 27, 2014 at 07:05:49PM +0200, Michael S. Tsirkin wrote:
>> apic polarity in KVM does not work: too many things assume active high.
>> Let's not pretend it works, let's just ignore polarity flag.  If we ever
>> want to emulate it exactly, this will need a feature flag anyway.
>>
>> Also report this to userspace: this makes it
>> possible to report the interrupt active-low
>> in ACPI, this way we are closer to real hardware.
>>
>> This patch fixes OSX running on KVM.
>>
>> Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>
>> ---
>>
>> Gabriel, could you confirm this fixes OSX for you?
>> If you can play with linux tweaking interrupt
>> to active low, that would be very nice too:
>> it's weekend here.
>
> With Fedora 20 Live x86_64:
>
> If all I do is 's/ActiveHigh/ActiveLow/' in hw/i386/[q36-]acpi-dsdt.dsl,
> but otherwise don't try to change how QEMU deals with "logical" vs.
> "physical" ioapic polarity, things work great. Printk's show polarity
> set to 1, but with the ignore-polarity patch things work fine.
>
> With normal (ActiveHigh) ACPI, printk reports polarity set to 0, and
> things *still* work exactly the same.
>
>
> So, the way I understand this (and I'm writing this mainly for myself,
> to make sure I understand correctly, so please kick me if I got it
> wrong), ACPI tells the guest OS how to configure "physical" ioapic polarity.
>
> With ActiveHigh, "physical" == "logical", i.e. "high" == "asserted"
> and "low" == "deasserted".
>
> With ActiveLow, "physical" == !"logical", so the other way around.
>
> QEMU being hard-coded to ActiveHigh is the moral equivalent of always
> sending the kernel (KVM) "logical" line states, rather than "physical"
> ones.
>
> Assuming KVM's userland clients are all coded for ActiveHigh, we can
> (should, for sanity's sake) just assume line states from userland are
> logical, and stop paying attention the polarity bits. That way,
> misbehaving guests [*] can configure their ioapics as they please, and
> things will just work OK regardless.
>
> As you pointed out earlier, even KVM itself already kind-of assumes
> ActiveHigh (e.g. in __kvm_irq_line_state(), which should be coded
> differently if ActiveLow were a serious possibility, and, BTW,
> irq_states[irq] would probably have to be initialized to all-1's if
> ActiveLow wre used, etc, etc).

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?

Also, there is a problem in this: we definitely do not want to have 
different ACPI tables for TCG vs. KVM.  Have you guys tested what 
happens with Linux guests + TCG if interrupts are declared active-low?

QEMU likely has many other places that hard-code active-high.  One 
approach could be to add a QOM property to the ioapic that is a bitmask 
of which GSIs are active-low.  The ioapic can consult it like this:

      if (vector >= 0 && vector < IOAPIC_NUM_PINS) {
          uint32_t mask = 1 << vector;
          uint64_t entry = s->ioredtbl[vector];

          if (entry & (1 << IOAPIC_LVT_POLARITY_SHIFT)) {
              level = !level;
          }
+        if (s->active_low_mask & (1 << vector)) {
+            level = !level;
+        }
          if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) ==
              IOAPIC_TRIGGER_LEVEL) {
              /* level triggered */

etc. so that the two NOTs undo each other, making the input to QEMU's 
ioapic also "logical" rather than "physical".

Paolo
Gabriel L. Somlo Feb. 27, 2014, 11:13 p.m. UTC | #3
On Thu, Feb 27, 2014 at 11:30:55PM +0100, Paolo Bonzini wrote:
> Il 27/02/2014 22:41, Gabriel L. Somlo ha scritto:
> >On Thu, Feb 27, 2014 at 07:05:49PM +0200, Michael S. Tsirkin wrote:
> >>apic polarity in KVM does not work: too many things assume active high.
> >>Let's not pretend it works, let's just ignore polarity flag.  If we ever
> >>want to emulate it exactly, this will need a feature flag anyway.
> >>
> >>Also report this to userspace: this makes it
> >>possible to report the interrupt active-low
> >>in ACPI, this way we are closer to real hardware.
> >>
> >>This patch fixes OSX running on KVM.
> >>
> >>Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
> >>Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>
> >>---
> >
> >So, the way I understand this (and I'm writing this mainly for myself,
> >to make sure I understand correctly, so please kick me if I got it
> >wrong), ACPI tells the guest OS how to configure "physical" ioapic polarity.
> >
> >With ActiveHigh, "physical" == "logical", i.e. "high" == "asserted"
> >and "low" == "deasserted".
> >
> >With ActiveLow, "physical" == !"logical", so the other way around.
> >
> >QEMU being hard-coded to ActiveHigh is the moral equivalent of always
> >sending the kernel (KVM) "logical" line states, rather than "physical"
> >ones.
> >
> >Assuming KVM's userland clients are all coded for ActiveHigh, we can
> >(should, for sanity's sake) just assume line states from userland are
> >logical, and stop paying attention the polarity bits. That way,
> >misbehaving guests [*] can configure their ioapics as they please, and
> >things will just work OK regardless.
> >
> >As you pointed out earlier, even KVM itself already kind-of assumes
> >ActiveHigh (e.g. in __kvm_irq_line_state(), which should be coded
> >differently if ActiveLow were a serious possibility, and, BTW,
> >irq_states[irq] would probably have to be initialized to all-1's if
> >ActiveLow wre used, etc, etc).
> 
> 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?

Do you mean one patch to change both virt/kvm/ioapic.c and
Documentation/virtual/kvm/api.txt ? Or a separate documentation patch ?
(sorry for my ignorance, I'm new to being a KVM contributor :) )

> >With Fedora 20 Live x86_64:
> >
> >If all I do is 's/ActiveHigh/ActiveLow/' in hw/i386/[q36-]acpi-dsdt.dsl,
> >but otherwise don't try to change how QEMU deals with "logical" vs.
> >"physical" ioapic polarity, things work great. Printk's show polarity
> >set to 1, but with the ignore-polarity patch things work fine.
> >
> >With normal (ActiveHigh) ACPI, printk reports polarity set to 0, and
> >things *still* work exactly the same.
>
> Also, there is a problem in this: we definitely do not want to have
> different ACPI tables for TCG vs. KVM.  Have you guys tested what
> happens with Linux guests + TCG if interrupts are declared
> active-low?

I think removing the polarity xor from KVM is about giving up on
trying to add ActiveLow support to QEMU altogether. What I tested
was what would happen if Linux (which pays attention to ACPI) were
told to use ActiveLow, but thre rest of QEMU continued being hardcoded
as ActiveHigh. Basically, another datapoint similar to what happens
with OS X, which completely ignores ACPI and configures the ioapic as
ActiveLow (even while running on ActiveHigh "hardware", i.e. QEMU).

With KVM no longer paying attention to the polarity bit, things work
fine, both with Linux-thinking-it's-ActiveLow, and with OS X.

But, since QEMU will stay ActiveHigh, I don't think TCG will be
impacted in any way by this change.

(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 :)

Thanks,
--Gabriel

> 
> QEMU likely has many other places that hard-code active-high.  One
> approach could be to add a QOM property to the ioapic that is a
> bitmask of which GSIs are active-low.  The ioapic can consult it
> like this:
> 
>      if (vector >= 0 && vector < IOAPIC_NUM_PINS) {
>          uint32_t mask = 1 << vector;
>          uint64_t entry = s->ioredtbl[vector];
> 
>          if (entry & (1 << IOAPIC_LVT_POLARITY_SHIFT)) {
>              level = !level;
>          }
> +        if (s->active_low_mask & (1 << vector)) {
> +            level = !level;
> +        }
>          if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) ==
>              IOAPIC_TRIGGER_LEVEL) {
>              /* level triggered */
> 
> etc. so that the two NOTs undo each other, making the input to
> QEMU's ioapic also "logical" rather than "physical".
> 
> Paolo
Paolo Bonzini Feb. 27, 2014, 11:31 p.m. UTC | #4
Il 28/02/2014 00:13, Gabriel L. Somlo ha scritto:
> On Thu, Feb 27, 2014 at 11:30:55PM +0100, Paolo Bonzini wrote:
>> Il 27/02/2014 22:41, Gabriel L. Somlo ha scritto:
>>> On Thu, Feb 27, 2014 at 07:05:49PM +0200, Michael S. Tsirkin wrote:
>>>> apic polarity in KVM does not work: too many things assume active high.
>>>> Let's not pretend it works, let's just ignore polarity flag.  If we ever
>>>> want to emulate it exactly, this will need a feature flag anyway.
>>>>
>>>> Also report this to userspace: this makes it
>>>> possible to report the interrupt active-low
>>>> in ACPI, this way we are closer to real hardware.
>>>>
>>>> This patch fixes OSX running on KVM.
>>>>
>>>> Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>
>>>> ---
>>>
>>> So, the way I understand this (and I'm writing this mainly for myself,
>>> to make sure I understand correctly, so please kick me if I got it
>>> wrong), ACPI tells the guest OS how to configure "physical" ioapic polarity.
>>>
>>> With ActiveHigh, "physical" == "logical", i.e. "high" == "asserted"
>>> and "low" == "deasserted".
>>>
>>> With ActiveLow, "physical" == !"logical", so the other way around.
>>>
>>> QEMU being hard-coded to ActiveHigh is the moral equivalent of always
>>> sending the kernel (KVM) "logical" line states, rather than "physical"
>>> ones.
>>>
>>> Assuming KVM's userland clients are all coded for ActiveHigh, we can
>>> (should, for sanity's sake) just assume line states from userland are
>>> logical, and stop paying attention the polarity bits. That way,
>>> misbehaving guests [*] can configure their ioapics as they please, and
>>> things will just work OK regardless.
>>>
>>> As you pointed out earlier, even KVM itself already kind-of assumes
>>> ActiveHigh (e.g. in __kvm_irq_line_state(), which should be coded
>>> differently if ActiveLow were a serious possibility, and, BTW,
>>> irq_states[irq] would probably have to be initialized to all-1's if
>>> ActiveLow wre used, etc, etc).
>>
>> 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?
>
> Do you mean one patch to change both virt/kvm/ioapic.c and
> Documentation/virtual/kvm/api.txt ? Or a separate documentation patch ?
> (sorry for my ignorance, I'm new to being a KVM contributor :) )

Yes.

> I think removing the polarity xor from KVM is about giving up on
> trying to add ActiveLow support to QEMU altogether. What I tested
> was what would happen if Linux (which pays attention to ACPI) were
> told to use ActiveLow, but thre rest of QEMU continued being hardcoded
> as ActiveHigh. Basically, another datapoint similar to what happens
> with OS X, which completely ignores ACPI and configures the ioapic as
> ActiveLow (even while running on ActiveHigh "hardware", i.e. QEMU).
>
> With KVM no longer paying attention to the polarity bit, things work
> fine, both with Linux-thinking-it's-ActiveLow, and with OS X.
>
> But, since QEMU will stay ActiveHigh, I don't think TCG will be
> impacted in any way by this change.

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

> (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. :)

Paolo
Michael S. Tsirkin Feb. 28, 2014, 4:55 a.m. UTC | #5
On Thu, Feb 27, 2014 at 11:30:55PM +0100, Paolo Bonzini wrote:
> Il 27/02/2014 22:41, Gabriel L. Somlo ha scritto:
> >On Thu, Feb 27, 2014 at 07:05:49PM +0200, Michael S. Tsirkin wrote:
> >>apic polarity in KVM does not work: too many things assume active high.
> >>Let's not pretend it works, let's just ignore polarity flag.  If we ever
> >>want to emulate it exactly, this will need a feature flag anyway.
> >>
> >>Also report this to userspace: this makes it
> >>possible to report the interrupt active-low
> >>in ACPI, this way we are closer to real hardware.
> >>
> >>This patch fixes OSX running on KVM.
> >>
> >>Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
> >>Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>
> >>---
> >>
> >>Gabriel, could you confirm this fixes OSX for you?
> >>If you can play with linux tweaking interrupt
> >>to active low, that would be very nice too:
> >>it's weekend here.
> >
> >With Fedora 20 Live x86_64:
> >
> >If all I do is 's/ActiveHigh/ActiveLow/' in hw/i386/[q36-]acpi-dsdt.dsl,
> >but otherwise don't try to change how QEMU deals with "logical" vs.
> >"physical" ioapic polarity, things work great. Printk's show polarity
> >set to 1, but with the ignore-polarity patch things work fine.
> >
> >With normal (ActiveHigh) ACPI, printk reports polarity set to 0, and
> >things *still* work exactly the same.
> >
> >
> >So, the way I understand this (and I'm writing this mainly for myself,
> >to make sure I understand correctly, so please kick me if I got it
> >wrong), ACPI tells the guest OS how to configure "physical" ioapic polarity.
> >
> >With ActiveHigh, "physical" == "logical", i.e. "high" == "asserted"
> >and "low" == "deasserted".
> >
> >With ActiveLow, "physical" == !"logical", so the other way around.
> >
> >QEMU being hard-coded to ActiveHigh is the moral equivalent of always
> >sending the kernel (KVM) "logical" line states, rather than "physical"
> >ones.
> >
> >Assuming KVM's userland clients are all coded for ActiveHigh, we can
> >(should, for sanity's sake) just assume line states from userland are
> >logical, and stop paying attention the polarity bits. That way,
> >misbehaving guests [*] can configure their ioapics as they please, and
> >things will just work OK regardless.
> >
> >As you pointed out earlier, even KVM itself already kind-of assumes
> >ActiveHigh (e.g. in __kvm_irq_line_state(), which should be coded
> >differently if ActiveLow were a serious possibility, and, BTW,
> >irq_states[irq] would probably have to be initialized to all-1's if
> >ActiveLow wre used, etc, etc).
> 
> 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?
> 
> Also, there is a problem in this: we definitely do not want to have
> different ACPI tables for TCG vs. KVM.

Why?

We already have different ACPI tables for CG vs KVM.
Specifically apic interrupt override flag in MADT is set
for KVM but not TCG.

As KVM hardware differs from TCG, I expect them to move
further apart with time.

> Have you guys tested what
> happens with Linux guests + TCG if interrupts are declared
> active-low?
> 
> QEMU likely has many other places that hard-code active-high.  One
> approach could be to add a QOM property to the ioapic that is a
> bitmask of which GSIs are active-low.  The ioapic can consult it
> like this:
> 
>      if (vector >= 0 && vector < IOAPIC_NUM_PINS) {
>          uint32_t mask = 1 << vector;
>          uint64_t entry = s->ioredtbl[vector];
> 
>          if (entry & (1 << IOAPIC_LVT_POLARITY_SHIFT)) {
>              level = !level;
>          }
> +        if (s->active_low_mask & (1 << vector)) {
> +            level = !level;
> +        }
>          if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) ==
>              IOAPIC_TRIGGER_LEVEL) {
>              /* level triggered */
> 
> etc. so that the two NOTs undo each other, making the input to
> QEMU's ioapic also "logical" rather than "physical".
> 
> Paolo

Or we can do a patch like we did for kvm and ignore polarity,
treating levels as logical rather than physical.
Paolo Bonzini Feb. 28, 2014, 8:10 a.m. UTC | #6
Il 28/02/2014 05:55, Michael S. Tsirkin ha scritto:
> Why?
>
> We already have different ACPI tables for TCG vs KVM.
> Specifically apic interrupt override flag in MADT is set
> for KVM but not TCG.

It used to be this way, but

   bool kvm_allows_irq0_override(void)
   {
       return !kvm_irqchip_in_kernel() || kvm_has_gsi_routing();
   }

means that these days it is usually set for both TCG and KVM.

> As KVM hardware differs from TCG, I expect them to move
> further apart with time.

I don't expect this, because there are some kind of guests that people 
will unknowingly end up running with TCG---for example libguestfs.  So 
minimizing the differences in hardware between KVM and TCG _is_ a goal.

> Or we can do a patch like we did for kvm and ignore polarity,
> treating levels as logical rather than physical.

Yes.

Paolo
Paolo Bonzini Feb. 28, 2014, 8:11 a.m. UTC | #7
Il 28/02/2014 05:55, Michael S. Tsirkin ha scritto:
> Why?
>
> We already have different ACPI tables for TCG vs KVM.
> Specifically apic interrupt override flag in MADT is set
> for KVM but not TCG.

It used to be this way, but

   bool kvm_allows_irq0_override(void)
   {
       return !kvm_irqchip_in_kernel() || kvm_has_gsi_routing();
   }

means that these days it is usually set for both TCG and KVM.

> As KVM hardware differs from TCG, I expect them to move
> further apart with time.

I don't expect this, because there are some kind of guests that people 
will unknowingly end up running with TCG---for example libguestfs.  So 
minimizing the differences in hardware between KVM and TCG _is_ a goal.

> Or we can do a patch like we did for kvm and ignore polarity,
> treating levels as logical rather than physical.

Yes.

Paolo
Alex Williamson March 1, 2014, 5:03 a.m. UTC | #8
On Thu, 2014-02-27 at 19:05 +0200, Michael S. Tsirkin wrote:
> apic polarity in KVM does not work: too many things assume active high.
> Let's not pretend it works, let's just ignore polarity flag.  If we ever
> want to emulate it exactly, this will need a feature flag anyway.
> 
> Also report this to userspace: this makes it
> possible to report the interrupt active-low
> in ACPI, this way we are closer to real hardware.
> 
> This patch fixes OSX running on KVM.
> 
> Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> 
> Gabriel, could you confirm this fixes OSX for you?
> If you can play with linux tweaking interrupt
> to active low, that would be very nice too:
> it's weekend here.
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 902f124..db29b27 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -674,6 +676,7 @@ struct kvm_ppc_smmu_info {
>  #define KVM_CAP_ARM_EL1_32BIT 93
>  #define KVM_CAP_SPAPR_MULTITCE 94
>  #define KVM_CAP_EXT_EMUL_CPUID 95
> +#define KVM_CAP_X86_IOAPIC_POLARITY_IGNORED 96
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 2d68297..ad170b4 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;

I've run Windows and Linux guests with vfio assigned devices with this
change and all the interrupt types seem to work.

Tested-by: Alex Williamson <alex.williamson@redhat.com>
diff mbox

Patch

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 902f124..db29b27 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -674,6 +676,7 @@  struct kvm_ppc_smmu_info {
 #define KVM_CAP_ARM_EL1_32BIT 93
 #define KVM_CAP_SPAPR_MULTITCE 94
 #define KVM_CAP_EXT_EMUL_CPUID 95
+#define KVM_CAP_X86_IOAPIC_POLARITY_IGNORED 96
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 2d68297..ad170b4 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;