>> I think you should avoid any changes to pci.c. Perhaps create a new
>> ioapic_and_pic_map / ioapic_and_pic_set pair of functions and change
>> pc.c to use that instead of piix_set_irq.
> I'll consider how to do this
>
But pci.c includes below code section, which implements irq_num mapping
through interrupt link, While ioapic_set_irq doesn't
need this modification.
Seems, it's unavoidable to modify pci.c
Thanks,
Anthony
pci_dev->irq_state[irq_num] = level;
for (;;) {
bus = pci_dev->bus;
irq_num = bus->map_irq(pci_dev, irq_num);
if (bus->set_irq)
break;
pci_dev = bus->parent_dev;
Xu, Anthony wrote:
> Marcelo Tosatti wrote:
>> Hi Anthony,
>>
>> On Fri, Jun 13, 2008 at 02:38:08PM +0800, Xu, Anthony wrote:
>>> Hi Avi and all
>>>
>>> This is the revised one,
>>>
>>> All PCI devices send interrupt to both PIC and IOAPIC,
>>> a). When PIC is enabled and IOAPIC is disabled, all redirect
>>> entries in IOAPIC are masked. B) When PIC is disabled and IPAPIC is
>>> enabled, link entry bit7 is set, means this link entry is disable.
>>> Guest OS need to guarantee PIC and IOAPIC are not enabled in the
>>> same time. Otherwise cause many suspicious interrupt to guest.
>>>
>>> Test by running guest linux in kvm/ia32 and kvm/ia64.
>>
>> Interrupt sharing is stable under Linux, PCI hotplug is happy, and
>> Windows is happy. Ship it!
> Thanks, I will
>
>>
>> I had to apply your patch by hand, your mailer eats newlines and
>> other nasty things, please fix that (or send attached patches).
>> Sorry, I'll attach patch.
>
>>
>>>
>>> + Name (PICD, 0)
>>>
>>> - /* PCI Bus definition */
>>> + Method(_PIC, 1)
>>> + {
>>> + Store(Arg0, PICD)
>>> + }
>>> +
>>> + /*PCI Bus definition */
>>
>> Why did you take off the space before the "P" of PCI? Before you ask
>> me, no, I don't have anything better to do :) typo
>
>>
>>> Scope(\_SB) {
>>> Device(PCI0) {
>>> Name (_HID, EisaId ("PNP0A03"))
>>> Name (_ADR, 0x00)
>>> Name (_UID, 1)
>>> - Name(_PRT, Package() {
>>> +
>>> + Method(_PRT,0){
>>> + If(PICD){
>>
>> Put some spaces there too.
> Okay
>
>>
>>> diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c
>>> index a23a466..f96fbb5 100644
>>> --- a/qemu/hw/pci.c
>>> +++ b/qemu/hw/pci.c
>>> @@ -27,6 +27,8 @@
>>> #include "net.h"
>>> #include "pc.h"
>>>
>>> +#include "qemu-kvm.h"
>>> +
>>> //#define DEBUG_PCI
>>>
>>> struct PCIBus {
>>> @@ -534,12 +536,18 @@ static void pci_set_irq(void *opaque, int
>>> irq_num, int level) PCIDevice *pci_dev = (PCIDevice *)opaque;
>>> PCIBus *bus; int change;
>>> -
>>> +#ifdef KVM_CAP_IRQCHIP
>>> + int irq;
>>> +#endif
>>> change = level - pci_dev->irq_state[irq_num];
>>> if (!change)
>>> return;
>>>
>>> pci_dev->irq_state[irq_num] = level;
>>> +#ifdef KVM_CAP_IRQCHIP
>>> + irq = ioapic_map_irq(pci_dev->devfn, irq_num);
>>> + ioapic_set_irq(opaque, irq, change);
>>> +#endif
>>
>> I think you should avoid any changes to pci.c. Perhaps create a new
>> ioapic_and_pic_map / ioapic_and_pic_set pair of functions and change
>> pc.c to use that instead of piix_set_irq.
> I'll consider how to do this
>
>>
>> Other than that (and KVM_CAP_IRQCHIP mentioned by Avi, along with
>> making sure this works with "-no-kvm") looks great. Agree
>
>>
>> Regarding the non-PIIX link devices I mentioned, that can be done
>> later if necessary.
> Agree, if we need to support dynamic irq, or support multiple IOAPIC.
>
>
>
>
> Anthony
--
To unsubscribe from this list: send the line "unsubscribe kvm-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
