Re: [Qemu-devel] [PATCH RFC] kvm: ignore apic polarity

[Paolo Bonzini]

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" <address@hidden>
> > > > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > > >
> > > > ---
> > >
> > > 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