Re: [PATCH RFC] kvm: ignore apic polarity

[Paolo Bonzini <pbonzini@redhat.com>]

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