From: "Gabriel L. Somlo" <gsomlo@gmail.com> To: Paolo Bonzini <pbonzini@redhat.com> Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "Michael S. Tsirkin" <mst@redhat.com>, "eddie.dong@intel.com" <eddie.dong@intel.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, Alexander Graf <agraf@suse.de>, Alex Williamson <alex.williamson@redhat.com> Subject: Re: [PATCH RFC] kvm: ignore apic polarity Date: Thu, 27 Feb 2014 18:13:12 -0500 [thread overview] Message-ID: <20140227231312.GH17184@ERROL.INI.CMU.EDU> (raw) In-Reply-To: <530FBC9F.8080800@redhat.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: "Gabriel L. Somlo" <gsomlo@gmail.com> To: Paolo Bonzini <pbonzini@redhat.com> Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "Michael S. Tsirkin" <mst@redhat.com>, "eddie.dong@intel.com" <eddie.dong@intel.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, Alexander Graf <agraf@suse.de>, Alex Williamson <alex.williamson@redhat.com> Subject: Re: [Qemu-devel] [PATCH RFC] kvm: ignore apic polarity Date: Thu, 27 Feb 2014 18:13:12 -0500 [thread overview] Message-ID: <20140227231312.GH17184@ERROL.INI.CMU.EDU> (raw) In-Reply-To: <530FBC9F.8080800@redhat.com> 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
next prev parent reply other threads:[~2014-02-27 23:13 UTC|newest] Thread overview: 155+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-01-08 20:02 [Qemu-devel] [PATCH] Add option to disable FDC from ISA bus and ACPI on i386 Gabriel L. Somlo 2014-01-08 20:38 ` Michael S. Tsirkin 2014-01-08 21:09 ` Gabriel L. Somlo 2014-01-15 11:13 ` Paolo Bonzini 2014-01-08 22:13 ` Paolo Bonzini 2014-01-08 23:39 ` Gabriel L. Somlo 2014-01-09 0:12 ` Alexander Graf 2014-01-09 1:51 ` Michael S. Tsirkin 2014-01-09 18:51 ` Gabriel L. Somlo 2014-01-09 20:12 ` Paolo Bonzini 2014-01-09 21:33 ` Michael S. Tsirkin 2014-01-09 21:58 ` Gabriel L. Somlo 2014-01-09 21:44 ` Gabriel L. Somlo 2014-01-10 12:37 ` Paolo Bonzini 2014-01-10 15:35 ` Gabriel L. Somlo 2014-01-10 16:13 ` Igor Mammedov 2014-01-17 21:10 ` [Qemu-devel] RFC: ACPI, HPET._CRS, MacOSX vs. WinXP Gabriel L. Somlo 2014-01-20 11:58 ` Michael S. Tsirkin 2014-01-20 11:57 ` Paolo Bonzini 2014-01-20 12:08 ` Michael S. Tsirkin 2014-01-20 12:16 ` Paolo Bonzini 2014-01-20 18:54 ` Gabriel L. Somlo 2014-01-20 20:31 ` Michael S. Tsirkin 2014-01-20 21:25 ` Gabriel L. Somlo 2014-01-21 10:33 ` Paolo Bonzini 2014-01-21 11:02 ` Michael S. Tsirkin 2014-01-21 11:05 ` Paolo Bonzini 2014-01-21 11:44 ` Michael S. Tsirkin 2014-01-21 18:11 ` [Qemu-devel] [PATCH] ACPI: Add IRQ resource to HPET._CRS on Mac OS X Gabriel L. Somlo 2014-01-21 18:38 ` Michael S. Tsirkin 2014-01-24 16:46 ` Gabriel L. Somlo 2014-01-24 21:18 ` Alexander Graf 2014-01-25 0:09 ` Gabriel L. Somlo 2014-01-25 9:08 ` Alexander Graf 2014-01-27 22:51 ` Gabriel L. Somlo 2014-01-27 23:51 ` Alexander Graf 2014-01-28 16:45 ` [Qemu-devel] OSX guest support review Gabriel L. Somlo 2014-01-28 16:57 ` Michael S. Tsirkin 2014-01-29 14:17 ` Alexander Graf 2014-01-29 21:36 ` [Qemu-devel] OSX guest vs. kvm ioapic polarity Gabriel L. Somlo 2014-01-29 22:18 ` Michael S. Tsirkin 2014-01-30 14:18 ` Gabriel L. Somlo 2014-01-30 19:48 ` Michael S. Tsirkin 2014-01-30 20:21 ` Gabriel L. Somlo 2014-01-30 20:28 ` Michael S. Tsirkin 2014-01-30 20:33 ` Michael S. Tsirkin 2014-01-30 20:44 ` Gabriel L. Somlo 2014-02-11 18:23 ` RFC: ioapic polarity vs. qemu os-x guest Gabriel L. Somlo 2014-02-11 18:23 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-11 19:54 ` Michael S. Tsirkin 2014-02-11 19:54 ` [Qemu-devel] " Michael S. Tsirkin 2014-02-11 21:35 ` Gabriel L. Somlo 2014-02-11 21:35 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-14 21:13 ` Gabriel L. Somlo 2014-02-14 21:13 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-14 21:21 ` Alexander Graf 2014-02-14 21:21 ` [Qemu-devel] " Alexander Graf 2014-02-14 22:06 ` Gabriel L. Somlo 2014-02-14 22:06 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-14 22:13 ` Alexander Graf 2014-02-14 22:13 ` [Qemu-devel] " Alexander Graf 2014-02-16 11:18 ` Michael S. Tsirkin 2014-02-16 11:18 ` [Qemu-devel] " Michael S. Tsirkin 2014-02-16 11:41 ` Michael S. Tsirkin 2014-02-16 11:41 ` [Qemu-devel] " Michael S. Tsirkin 2014-02-16 14:47 ` Alex Williamson 2014-02-16 14:47 ` [Qemu-devel] " Alex Williamson 2014-02-16 16:23 ` Michael S. Tsirkin 2014-02-16 16:23 ` [Qemu-devel] " Michael S. Tsirkin 2014-02-17 17:57 ` Gabriel L. Somlo 2014-02-17 17:57 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-17 18:01 ` Gabriel L. Somlo 2014-02-17 18:01 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-17 18:06 ` Paolo Bonzini 2014-02-17 18:06 ` [Qemu-devel] " Paolo Bonzini 2014-02-17 19:38 ` Gabriel L. Somlo 2014-02-17 19:38 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-18 0:58 ` Gabriel L. Somlo 2014-02-18 0:58 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-27 17:05 ` [PATCH RFC] kvm: ignore apic polarity Michael S. Tsirkin 2014-02-27 17:05 ` [Qemu-devel] " Michael S. Tsirkin 2014-02-27 21:41 ` Gabriel L. Somlo 2014-02-27 21:41 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-27 22:30 ` Paolo Bonzini 2014-02-27 22:30 ` [Qemu-devel] " Paolo Bonzini 2014-02-27 23:13 ` Gabriel L. Somlo [this message] 2014-02-27 23:13 ` Gabriel L. Somlo 2014-02-27 23:31 ` Paolo Bonzini 2014-02-27 23:31 ` [Qemu-devel] " Paolo Bonzini 2014-02-28 4:06 ` [RFC PATCH v2] kvm: x86: ignore ioapic polarity Gabriel L. Somlo 2014-02-28 4:06 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-28 17:23 ` [Qemu-devel] [RFC PATCH] qemu: " Gabriel L. Somlo 2014-02-28 18:57 ` [Qemu-devel] [RFC PATCH v2] " Gabriel L. Somlo 2014-02-28 19:14 ` [Qemu-devel] [PATCH] qemu: x86: report lapic version as 0x14 instead of 0x11 Gabriel L. Somlo 2014-03-01 3:44 ` Alexander Graf 2014-03-01 4:25 ` Gabriel L. Somlo 2014-03-01 5:45 ` Alexander Graf 2014-03-01 14:46 ` Paolo Bonzini 2014-03-02 0:17 ` Gabriel L. Somlo 2014-03-02 8:56 ` Paolo Bonzini 2014-03-02 14:31 ` Michael S. Tsirkin 2014-03-02 16:02 ` Michael S. Tsirkin 2014-03-06 7:50 ` Michael S. Tsirkin 2014-03-02 14:52 ` [Qemu-devel] [RFC PATCH v2] qemu: x86: ignore ioapic polarity Michael S. Tsirkin 2014-03-02 16:15 ` Gabriel L. Somlo 2014-03-02 17:09 ` Michael S. Tsirkin 2014-03-06 7:45 ` Michael S. Tsirkin 2014-03-02 14:55 ` [RFC PATCH v2] kvm: " Michael S. Tsirkin 2014-03-02 14:55 ` [Qemu-devel] " Michael S. Tsirkin 2014-03-13 10:53 ` Paolo Bonzini 2014-03-13 10:53 ` [Qemu-devel] " Paolo Bonzini 2014-03-13 13:43 ` Gabriel L. Somlo 2014-03-13 13:43 ` [Qemu-devel] " Gabriel L. Somlo 2014-02-28 4:55 ` [PATCH RFC] kvm: ignore apic polarity Michael S. Tsirkin 2014-02-28 4:55 ` [Qemu-devel] " Michael S. Tsirkin 2014-02-28 8:10 ` Paolo Bonzini 2014-02-28 8:10 ` [Qemu-devel] " Paolo Bonzini 2014-02-28 8:11 ` Paolo Bonzini 2014-02-28 8:11 ` [Qemu-devel] " Paolo Bonzini 2014-03-01 5:03 ` Alex Williamson 2014-03-01 5:03 ` [Qemu-devel] " Alex Williamson 2014-02-16 11:34 ` RFC: ioapic polarity vs. qemu os-x guest Michael S. Tsirkin 2014-02-16 11:34 ` [Qemu-devel] " Michael S. Tsirkin 2014-02-16 15:12 ` Peter Maydell 2014-02-16 11:37 ` Michael S. Tsirkin 2014-02-16 11:37 ` [Qemu-devel] " Michael S. Tsirkin 2014-01-29 23:13 ` [Qemu-devel] OSX guest vs. kvm ioapic polarity Alexander Graf 2014-01-30 15:56 ` Gabriel L. Somlo 2014-01-28 20:40 ` [Qemu-devel] osx bootloader Gabriel L. Somlo 2014-01-28 22:51 ` BALATON Zoltan 2014-01-29 3:07 ` Gabriel L. Somlo 2014-01-29 11:29 ` BALATON Zoltan 2014-01-29 14:53 ` Gabriel L. Somlo 2014-01-29 15:00 ` Alexander Graf 2014-01-30 0:15 ` BALATON Zoltan 2014-02-01 17:43 ` BALATON Zoltan 2014-02-01 0:38 ` BALATON Zoltan 2014-02-01 10:07 ` Alexander Graf 2014-02-01 14:35 ` [Qemu-devel] OVMF with q35 (was: osx bootloader) BALATON Zoltan 2014-02-01 15:13 ` Alexander Graf 2014-02-01 17:38 ` BALATON Zoltan 2014-02-03 7:47 ` Gerd Hoffmann 2014-02-01 21:19 ` [Qemu-devel] osx bootloader Paolo Bonzini 2014-02-02 2:18 ` BALATON Zoltan 2014-01-29 12:10 ` BALATON Zoltan 2014-01-29 14:20 ` Alexander Graf 2014-01-21 11:38 ` [Qemu-devel] RFC: ACPI, HPET._CRS, MacOSX vs. WinXP Michael S. Tsirkin 2014-01-20 20:23 ` Michael S. Tsirkin 2014-01-20 12:03 ` Igor Mammedov 2014-01-09 8:46 ` [Qemu-devel] [PATCH] Add option to disable FDC from ISA bus and ACPI on i386 Markus Armbruster 2014-01-09 10:24 ` Paolo Bonzini 2014-01-31 19:03 ` [Qemu-devel] [RFC PATCH v2] Add option to switch off FDC Gabriel L. Somlo 2014-01-31 19:39 ` Eric Blake 2014-02-02 13:47 ` Michael S. Tsirkin 2014-02-10 14:10 ` Marcel Apfelbaum
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20140227231312.GH17184@ERROL.INI.CMU.EDU \ --to=gsomlo@gmail.com \ --cc=agraf@suse.de \ --cc=alex.williamson@redhat.com \ --cc=eddie.dong@intel.com \ --cc=kvm@vger.kernel.org \ --cc=mst@redhat.com \ --cc=pbonzini@redhat.com \ --cc=qemu-devel@nongnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.