* [Qemu-devel] [PATCH] fix bug of isa_bus irq
@ 2012-03-11 7:04 Wanpeng Li
2012-03-11 7:46 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Wanpeng Li @ 2012-03-11 7:04 UTC (permalink / raw)
To: Michael S. Tsirkin, Anthony Liguori, qemu-devel
Cc: Wanpeng Li, afaerber, Gavin Shan
ISA bus only use IRQ 0~15, so don't need to give an array qemu_irq 0~23, just
array qemu_irq i8259 is ok.
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
---
hw/pc_piix.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 63dba4e..52f7cf8 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -210,7 +210,6 @@ static void pc_init1(MemoryRegion *system_memory,
isa_bus = isa_bus_new(NULL, system_io);
no_hpet = 1;
}
- isa_bus_irqs(isa_bus, gsi);
if (kvm_irqchip_in_kernel()) {
i8259 = kvm_i8259_init(isa_bus);
@@ -221,6 +220,8 @@ static void pc_init1(MemoryRegion *system_memory,
i8259 = i8259_init(isa_bus, cpu_irq[0]);
}
+ isa_bus_irqs(isa_bus, i8259);
+
for (i = 0; i < ISA_NUM_IRQS; i++) {
gsi_state->i8259_irq[i] = i8259[i];
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] fix bug of isa_bus irq
2012-03-11 7:04 [Qemu-devel] [PATCH] fix bug of isa_bus irq Wanpeng Li
@ 2012-03-11 7:46 ` Jan Kiszka
2012-03-12 6:08 ` Wanpeng Li
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2012-03-11 7:46 UTC (permalink / raw)
To: Wanpeng Li
Cc: Gavin Shan, Anthony Liguori, qemu-devel, afaerber,
Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]
On 2012-03-11 08:04, Wanpeng Li wrote:
> ISA bus only use IRQ 0~15, so don't need to give an array qemu_irq 0~23, just
> array qemu_irq i8259 is ok.
>
> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
> ---
> hw/pc_piix.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 63dba4e..52f7cf8 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -210,7 +210,6 @@ static void pc_init1(MemoryRegion *system_memory,
> isa_bus = isa_bus_new(NULL, system_io);
> no_hpet = 1;
> }
> - isa_bus_irqs(isa_bus, gsi);
>
> if (kvm_irqchip_in_kernel()) {
> i8259 = kvm_i8259_init(isa_bus);
> @@ -221,6 +220,8 @@ static void pc_init1(MemoryRegion *system_memory,
> i8259 = i8259_init(isa_bus, cpu_irq[0]);
> }
>
> + isa_bus_irqs(isa_bus, i8259);
> +
> for (i = 0; i < ISA_NUM_IRQS; i++) {
> gsi_state->i8259_irq[i] = i8259[i];
> }
This is bogus. isa_bus_irqs sets the output IRQs of the ISA bus. And
those are not only delivered to the PIC on the PIIX2, but also the
IOAPIC. Thus we have to pass in the GSI input lines which dispatch to
both. Of those lines, only the first 16 will be used by the ISA bus
(there is even an assert to ensure this).
Did you see any concrete bug in the context of this logic?
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] fix bug of isa_bus irq
2012-03-11 7:46 ` Jan Kiszka
@ 2012-03-12 6:08 ` Wanpeng Li
2012-03-12 6:55 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Wanpeng Li @ 2012-03-12 6:08 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel
On Sun, Mar 11, 2012 at 08:46:38AM +0100, Jan Kiszka wrote:
>On 2012-03-11 08:04, Wanpeng Li wrote:
>> ISA bus only use IRQ 0~15, so don't need to give an array qemu_irq 0~23, just
>> array qemu_irq i8259 is ok.
>>
>> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
>> ---
>> hw/pc_piix.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>> index 63dba4e..52f7cf8 100644
>> --- a/hw/pc_piix.c
>> +++ b/hw/pc_piix.c
>> @@ -210,7 +210,6 @@ static void pc_init1(MemoryRegion *system_memory,
>> isa_bus = isa_bus_new(NULL, system_io);
>> no_hpet = 1;
>> }
>> - isa_bus_irqs(isa_bus, gsi);
>>
>> if (kvm_irqchip_in_kernel()) {
>> i8259 = kvm_i8259_init(isa_bus);
>> @@ -221,6 +220,8 @@ static void pc_init1(MemoryRegion *system_memory,
>> i8259 = i8259_init(isa_bus, cpu_irq[0]);
>> }
>>
>> + isa_bus_irqs(isa_bus, i8259);
>> +
>> for (i = 0; i < ISA_NUM_IRQS; i++) {
>> gsi_state->i8259_irq[i] = i8259[i];
>> }
>
>This is bogus. isa_bus_irqs sets the output IRQs of the ISA bus. And
>those are not only delivered to the PIC on the PIIX2, but also the
>IOAPIC. Thus we have to pass in the GSI input lines which dispatch to
>both. Of those lines, only the first 16 will be used by the ISA bus
>(there is even an assert to ensure this).
>
>Did you see any concrete bug in the context of this logic?
>
>Jan
>
Yes, but actually PIC is being used at present, whether passing qemu_irq
0~23 to isa_bus is not safe or not.
Wanpeng Li
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] fix bug of isa_bus irq
2012-03-12 6:08 ` Wanpeng Li
@ 2012-03-12 6:55 ` Jan Kiszka
0 siblings, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2012-03-12 6:55 UTC (permalink / raw)
To: Wanpeng Li; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2048 bytes --]
On 2012-03-12 07:08, Wanpeng Li wrote:
> On Sun, Mar 11, 2012 at 08:46:38AM +0100, Jan Kiszka wrote:
>> On 2012-03-11 08:04, Wanpeng Li wrote:
>>> ISA bus only use IRQ 0~15, so don't need to give an array qemu_irq 0~23, just
>>> array qemu_irq i8259 is ok.
>>>
>>> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
>>> ---
>>> hw/pc_piix.c | 3 ++-
>>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>>> index 63dba4e..52f7cf8 100644
>>> --- a/hw/pc_piix.c
>>> +++ b/hw/pc_piix.c
>>> @@ -210,7 +210,6 @@ static void pc_init1(MemoryRegion *system_memory,
>>> isa_bus = isa_bus_new(NULL, system_io);
>>> no_hpet = 1;
>>> }
>>> - isa_bus_irqs(isa_bus, gsi);
>>>
>>> if (kvm_irqchip_in_kernel()) {
>>> i8259 = kvm_i8259_init(isa_bus);
>>> @@ -221,6 +220,8 @@ static void pc_init1(MemoryRegion *system_memory,
>>> i8259 = i8259_init(isa_bus, cpu_irq[0]);
>>> }
>>>
>>> + isa_bus_irqs(isa_bus, i8259);
>>> +
>>> for (i = 0; i < ISA_NUM_IRQS; i++) {
>>> gsi_state->i8259_irq[i] = i8259[i];
>>> }
>>
>> This is bogus. isa_bus_irqs sets the output IRQs of the ISA bus. And
>> those are not only delivered to the PIC on the PIIX2, but also the
>> IOAPIC. Thus we have to pass in the GSI input lines which dispatch to
>> both. Of those lines, only the first 16 will be used by the ISA bus
>> (there is even an assert to ensure this).
>>
>> Did you see any concrete bug in the context of this logic?
>>
>> Jan
>>
>
> Yes, but actually PIC is being used at present, whether passing qemu_irq
> 0~23 to isa_bus is not safe or not.
Sorry, IRQ routing to PIC and IOAPIC is actually not a property of the
PIIX3 but the board we emulate. And here we follow the Multiprocessor
Specification of Intel and route ISA bus IRQs to both interrupt
controllers. Thus the bus must be connected to the GSIs. And, again,
GSI[16..13] aren't referenced by the ISA bus at any time.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH] fix bug of isa_bus irq
@ 2012-03-11 6:39 Wanpeng Li
0 siblings, 0 replies; 5+ messages in thread
From: Wanpeng Li @ 2012-03-11 6:39 UTC (permalink / raw)
To: Michael S. Tsirkin, Anthony Liguori, qemu-devel
Cc: Wanpeng Li, afaerber, Gavin Shan
ISA bus only use IRQ 0~15, so don't need to give an array qemu_irq 0~24, just
array qemu_irq i8259 is ok.
Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
---
hw/pc_piix.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 63dba4e..52f7cf8 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -210,7 +210,6 @@ static void pc_init1(MemoryRegion *system_memory,
isa_bus = isa_bus_new(NULL, system_io);
no_hpet = 1;
}
- isa_bus_irqs(isa_bus, gsi);
if (kvm_irqchip_in_kernel()) {
i8259 = kvm_i8259_init(isa_bus);
@@ -221,6 +220,8 @@ static void pc_init1(MemoryRegion *system_memory,
i8259 = i8259_init(isa_bus, cpu_irq[0]);
}
+ isa_bus_irqs(isa_bus, i8259);
+
for (i = 0; i < ISA_NUM_IRQS; i++) {
gsi_state->i8259_irq[i] = i8259[i];
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-03-12 6:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-11 7:04 [Qemu-devel] [PATCH] fix bug of isa_bus irq Wanpeng Li
2012-03-11 7:46 ` Jan Kiszka
2012-03-12 6:08 ` Wanpeng Li
2012-03-12 6:55 ` Jan Kiszka
-- strict thread matches above, loose matches on Subject: below --
2012-03-11 6:39 Wanpeng Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).