[PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

[PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

[TaiseiIto]

Before this commit, interruptions from i8254 which should be sent to IRQ0
were sent to IRQ2. After this commit, these are correctly sent to IRQ0. When
I had an HPET timer generate interruptions once per second to test an HPET
driver in my operating system on QEMU, I observed more frequent
interruptions than I configured on the HPET timer. I investigated the cause
and found that not only interruptions from HPET but also interruptions from
i8254 were sent to IRQ2 because of a redirection from IRQ0 to IRQ2. This
redirection is added in hw/apic.c at commit
16b29ae1807b024bd5052301550f5d47dae958a2 but this redirection caused wrong
interruptions. So I deleted the redirection. Finally, I confirmed there is
no problem on 'make check' results and that interruptions from i8254 and
interruptions from HPET are correclty sent to IRQ0 and IRQ2 respectively.

Signed-off-by: TaiseiIto <taisei1212@outlook.jp>
---
 hw/intc/ioapic.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 716ffc8bbb..6b630b45ca 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -154,15 +154,8 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
 {
     IOAPICCommonState *s = opaque;
 
-    /* ISA IRQs map to GSI 1-1 except for IRQ0 which maps
-     * to GSI 2.  GSI maps to ioapic 1-1.  This is not
-     * the cleanest way of doing it but it should work. */
-
     trace_ioapic_set_irq(vector, level);
     ioapic_stat_update_irq(s, vector, level);
-    if (vector == 0) {
-        vector = 2;
-    }
     if (vector < IOAPIC_NUM_PINS) {
         uint32_t mask = 1 << vector;
         uint64_t entry = s->ioredtbl[vector];
-- 
2.34.1

[PING][PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

[伊藤 太清]

[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]

This is a ping to the patch below.
https://lore.kernel.org/qemu-devel/TY0PR0101MB42850337F8917D1F514107FBA4D52@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/

________________________________
差出人: TaiseiIto <taisei1212@outlook.jp>
送信日時: 2024年6月25日 21:03
宛先: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
CC: pbonzini@redhat.com <pbonzini@redhat.com>; mst@redhat.com <mst@redhat.com>; TaiseiIto <taisei1212@outlook.jp>
件名: [PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

Before this commit, interruptions from i8254 which should be sent to IRQ0
were sent to IRQ2. After this commit, these are correctly sent to IRQ0. When
I had an HPET timer generate interruptions once per second to test an HPET
driver in my operating system on QEMU, I observed more frequent
interruptions than I configured on the HPET timer. I investigated the cause
and found that not only interruptions from HPET but also interruptions from
i8254 were sent to IRQ2 because of a redirection from IRQ0 to IRQ2. This
redirection is added in hw/apic.c at commit
16b29ae1807b024bd5052301550f5d47dae958a2 but this redirection caused wrong
interruptions. So I deleted the redirection. Finally, I confirmed there is
no problem on 'make check' results and that interruptions from i8254 and
interruptions from HPET are correclty sent to IRQ0 and IRQ2 respectively.

Signed-off-by: TaiseiIto <taisei1212@outlook.jp>
---
 hw/intc/ioapic.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 716ffc8bbb..6b630b45ca 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -154,15 +154,8 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
 {
     IOAPICCommonState *s = opaque;

-    /* ISA IRQs map to GSI 1-1 except for IRQ0 which maps
-     * to GSI 2.  GSI maps to ioapic 1-1.  This is not
-     * the cleanest way of doing it but it should work. */
-
     trace_ioapic_set_irq(vector, level);
     ioapic_stat_update_irq(s, vector, level);
-    if (vector == 0) {
-        vector = 2;
-    }
     if (vector < IOAPIC_NUM_PINS) {
         uint32_t mask = 1 << vector;
         uint64_t entry = s->ioredtbl[vector];
--
2.34.1


[-- Attachment #2: Type: text/html, Size: 3999 bytes --]

[PING^2][PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

[伊藤 太清]

[-- Attachment #1: Type: text/plain, Size: 2817 bytes --]

This is a ping to the patch below.
https://lore.kernel.org/qemu-devel/TY0PR0101MB42850337F8917D1F514107FBA4D52@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/

________________________________
差出人: 伊藤 太清 <taisei1212@outlook.jp>
送信日時: 2024年7月5日 12:33
宛先: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
CC: pbonzini@redhat.com <pbonzini@redhat.com>; mst@redhat.com <mst@redhat.com>
件名: [PING][PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

This is a ping to the patch below.
https://lore.kernel.org/qemu-devel/TY0PR0101MB42850337F8917D1F514107FBA4D52@TY0PR0101MB4285.apcprd01.prod.exchangelabs.com/

________________________________
差出人: TaiseiIto <taisei1212@outlook.jp>
送信日時: 2024年6月25日 21:03
宛先: qemu-devel@nongnu.org <qemu-devel@nongnu.org>
CC: pbonzini@redhat.com <pbonzini@redhat.com>; mst@redhat.com <mst@redhat.com>; TaiseiIto <taisei1212@outlook.jp>
件名: [PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

Before this commit, interruptions from i8254 which should be sent to IRQ0
were sent to IRQ2. After this commit, these are correctly sent to IRQ0. When
I had an HPET timer generate interruptions once per second to test an HPET
driver in my operating system on QEMU, I observed more frequent
interruptions than I configured on the HPET timer. I investigated the cause
and found that not only interruptions from HPET but also interruptions from
i8254 were sent to IRQ2 because of a redirection from IRQ0 to IRQ2. This
redirection is added in hw/apic.c at commit
16b29ae1807b024bd5052301550f5d47dae958a2 but this redirection caused wrong
interruptions. So I deleted the redirection. Finally, I confirmed there is
no problem on 'make check' results and that interruptions from i8254 and
interruptions from HPET are correclty sent to IRQ0 and IRQ2 respectively.

Signed-off-by: TaiseiIto <taisei1212@outlook.jp>
---
 hw/intc/ioapic.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 716ffc8bbb..6b630b45ca 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -154,15 +154,8 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
 {
     IOAPICCommonState *s = opaque;

-    /* ISA IRQs map to GSI 1-1 except for IRQ0 which maps
-     * to GSI 2.  GSI maps to ioapic 1-1.  This is not
-     * the cleanest way of doing it but it should work. */
-
     trace_ioapic_set_irq(vector, level);
     ioapic_stat_update_irq(s, vector, level);
-    if (vector == 0) {
-        vector = 2;
-    }
     if (vector < IOAPIC_NUM_PINS) {
         uint32_t mask = 1 << vector;
         uint64_t entry = s->ioredtbl[vector];
--
2.34.1


[-- Attachment #2: Type: text/html, Size: 5718 bytes --]

Re: [PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

[Paolo Bonzini]

On Tue, Jun 25, 2024 at 2:03 PM TaiseiIto <taisei1212@outlook.jp> wrote:
> Before this commit, interruptions from i8254 which should be sent to IRQ0
> were sent to IRQ2. After this commit, these are correctly sent to IRQ0. When
> I had an HPET timer generate interruptions once per second to test an HPET
> driver in my operating system on QEMU, I observed more frequent
> interruptions than I configured on the HPET timer. I investigated the cause
> and found that not only interruptions from HPET but also interruptions from
> i8254 were sent to IRQ2 because of a redirection from IRQ0 to IRQ2. This
> redirection is added in hw/apic.c at commit
> 16b29ae1807b024bd5052301550f5d47dae958a2 but this redirection caused wrong
> interruptions. So I deleted the redirection. Finally, I confirmed there is
> no problem on 'make check' results and that interruptions from i8254 and
> interruptions from HPET are correclty sent to IRQ0 and IRQ2 respectively.

Hi, did you set the legacy replacement route bit on the HPET?

My understanding is that:
- if you enable legacy-replacement routing, the HPET will take care of
dropping all i8254 interrupts
- if you disable legacy-replacement routing, the i8254 will still
generate interrupts on ISA IRQ 0. If you then enable the IO-APIC and
program the routes according to the ACPI MADT table, the interrupt
from the i8254 (ISA IRQ 0) will be redirected to the IO-APIC's GSI2.

So the solutions would be one of the following if you use HPET timer 0:
- disable the i8254
- enable legacy-replacement routing
- mask GSI2 on the IO-APIC and use a different route for the HPET
(worse, but should also work)

Thanks,

Paolo


> Signed-off-by: TaiseiIto <taisei1212@outlook.jp>
> ---
>  hw/intc/ioapic.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
> index 716ffc8bbb..6b630b45ca 100644
> --- a/hw/intc/ioapic.c
> +++ b/hw/intc/ioapic.c
> @@ -154,15 +154,8 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
>  {
>      IOAPICCommonState *s = opaque;
>
> -    /* ISA IRQs map to GSI 1-1 except for IRQ0 which maps
> -     * to GSI 2.  GSI maps to ioapic 1-1.  This is not
> -     * the cleanest way of doing it but it should work. */
> -
>      trace_ioapic_set_irq(vector, level);
>      ioapic_stat_update_irq(s, vector, level);
> -    if (vector == 0) {
> -        vector = 2;
> -    }
>      if (vector < IOAPIC_NUM_PINS) {
>          uint32_t mask = 1 << vector;
>          uint64_t entry = s->ioredtbl[vector];
> --
> 2.34.1
>

Re: [PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

[伊藤 太清]

[-- Attachment #1: Type: text/plain, Size: 3041 bytes --]

I tried to enable the legacy replacement routing without the patch.
It works correctly.
Thanks.

Taisei

________________________________
差出人: Paolo Bonzini <pbonzini@redhat.com>
送信日時: 2024年7月23日 23:15
宛先: TaiseiIto <taisei1212@outlook.jp>
CC: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; mst@redhat.com <mst@redhat.com>
件名: Re: [PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

On Tue, Jun 25, 2024 at 2:03 PM TaiseiIto <taisei1212@outlook.jp> wrote:
> Before this commit, interruptions from i8254 which should be sent to IRQ0
> were sent to IRQ2. After this commit, these are correctly sent to IRQ0. When
> I had an HPET timer generate interruptions once per second to test an HPET
> driver in my operating system on QEMU, I observed more frequent
> interruptions than I configured on the HPET timer. I investigated the cause
> and found that not only interruptions from HPET but also interruptions from
> i8254 were sent to IRQ2 because of a redirection from IRQ0 to IRQ2. This
> redirection is added in hw/apic.c at commit
> 16b29ae1807b024bd5052301550f5d47dae958a2 but this redirection caused wrong
> interruptions. So I deleted the redirection. Finally, I confirmed there is
> no problem on 'make check' results and that interruptions from i8254 and
> interruptions from HPET are correclty sent to IRQ0 and IRQ2 respectively.

Hi, did you set the legacy replacement route bit on the HPET?

My understanding is that:
- if you enable legacy-replacement routing, the HPET will take care of
dropping all i8254 interrupts
- if you disable legacy-replacement routing, the i8254 will still
generate interrupts on ISA IRQ 0. If you then enable the IO-APIC and
program the routes according to the ACPI MADT table, the interrupt
from the i8254 (ISA IRQ 0) will be redirected to the IO-APIC's GSI2.

So the solutions would be one of the following if you use HPET timer 0:
- disable the i8254
- enable legacy-replacement routing
- mask GSI2 on the IO-APIC and use a different route for the HPET
(worse, but should also work)

Thanks,

Paolo


> Signed-off-by: TaiseiIto <taisei1212@outlook.jp>
> ---
>  hw/intc/ioapic.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
> index 716ffc8bbb..6b630b45ca 100644
> --- a/hw/intc/ioapic.c
> +++ b/hw/intc/ioapic.c
> @@ -154,15 +154,8 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
>  {
>      IOAPICCommonState *s = opaque;
>
> -    /* ISA IRQs map to GSI 1-1 except for IRQ0 which maps
> -     * to GSI 2.  GSI maps to ioapic 1-1.  This is not
> -     * the cleanest way of doing it but it should work. */
> -
>      trace_ioapic_set_irq(vector, level);
>      ioapic_stat_update_irq(s, vector, level);
> -    if (vector == 0) {
> -        vector = 2;
> -    }
>      if (vector < IOAPIC_NUM_PINS) {
>          uint32_t mask = 1 << vector;
>          uint64_t entry = s->ioredtbl[vector];
> --
> 2.34.1
>


[-- Attachment #2: Type: text/html, Size: 5142 bytes --]

Re: [PATCH] hw/intc/ioapic: Delete a wrong IRQ redirection on I/O APIC

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

On Tue, 2024-07-23 at 16:15 +0200, Paolo Bonzini wrote:
> On Tue, Jun 25, 2024 at 2:03 PM TaiseiIto <taisei1212@outlook.jp> wrote:
> > Before this commit, interruptions from i8254 which should be sent to IRQ0
> > were sent to IRQ2. After this commit, these are correctly sent to IRQ0. When
> > I had an HPET timer generate interruptions once per second to test an HPET
> > driver in my operating system on QEMU, I observed more frequent
> > interruptions than I configured on the HPET timer. I investigated the cause
> > and found that not only interruptions from HPET but also interruptions from
> > i8254 were sent to IRQ2 because of a redirection from IRQ0 to IRQ2. This
> > redirection is added in hw/apic.c at commit
> > 16b29ae1807b024bd5052301550f5d47dae958a2 but this redirection caused wrong
> > interruptions. So I deleted the redirection. Finally, I confirmed there is
> > no problem on 'make check' results and that interruptions from i8254 and
> > interruptions from HPET are correclty sent to IRQ0 and IRQ2 respectively.
> 
> Hi, did you set the legacy replacement route bit on the HPET?
> 
> My understanding is that:
> - if you enable legacy-replacement routing, the HPET will take care of
> dropping all i8254 interrupts
> - if you disable legacy-replacement routing, the i8254 will still
> generate interrupts on ISA IRQ 0. If you then enable the IO-APIC and
> program the routes according to the ACPI MADT table, the interrupt
> from the i8254 (ISA IRQ 0) will be redirected to the IO-APIC's GSI2.
> 
> So the solutions would be one of the following if you use HPET timer 0:
> - disable the i8254
> - enable legacy-replacement routing
> - mask GSI2 on the IO-APIC and use a different route for the HPET
> (worse, but should also work)

Or we delete the existing hack which redirects IRQ0→IRQ2 in the I/OAPIC
and replace it with an IRQ2→IRQ0 hack in gsi_handler(), as seen in
https://mail.gnu.org/archive/html/qemu-devel/2023-10/msg06163.html

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]