[0.14?][PATCH v3 0/4] IOAPIC fixes
Third round, primarily dropping IOAPIC relocation support:
- replace IOAPIC relocation with a qemu-kvm compat patch
- fix ioapic_eoi_broadcast name
Note that, even if we do not include the base address explicitly in the IOAPIC state, we will still need to read it out for KVM in-kernel support as its ABI considers the base address part of the IOAPIC state.
Jan Kiszka (4):
ioapic: Implement EOI handling for level-triggered IRQs
ioapic: Save/restore irr
ioapic: Add support for qemu-kvm's vmstate v2
ioapic: Style & magics cleanup
hw/apic.c | 9 ++- hw/ioapic.c | 230 +++++++++++++++++++++++++++++++++++++++++------------------ hw/ioapic.h | 20 +++++ 3 files changed, 186 insertions(+), 73 deletions(-) create mode 100644 hw/ioapic.h
[0.14?][PATCH v3 1/4] ioapic: Implement EOI handling for level-triggered IRQs
From: Jan Kiszka <[email protected]>
Add the missing EOI broadcast from local APIC to the IOAPICs on completion of level-triggered IRQs. This ensures that a still asserted IRQ source properly re-triggers an APIC IRQ.
--- hw/apic.c | 9 ++++++--- hw/ioapic.c | 43 +++++++++++++++++++++++++++++++++++++++++-- hw/ioapic.h | 20 ++++++++++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 hw/ioapic.h
hw/apic.c +6 -3
@@ -18,6 +18,7 @@*/ #include "hw.h" #include "apic.h" +#include "ioapic.h" #include "qemu-timer.h" #include "host-utils.h" #include "sysbus.h"
@@ -57,7 +58,8 @@#define ESR_ILLEGAL_ADDRESS (1 << 7) -#define APIC_SV_ENABLE (1 << 8) +#define APIC_SV_DIRECTED_IO (1<<12) +#define APIC_SV_ENABLE (1<<8)#define MAX_APICS 255#define MAX_APIC_WORDS 8
@@ -420,8 +422,9 @@ static void apic_eoi(APICState *s)if (isrv <0 )return ;reset_bit (s->isr, isrv); -/* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to -set the remote IRR bit for level triggered interrupts. */ +if (!(s->spurious_vec &APIC_SV_DIRECTED_IO ) &&get_bit (s->tmr, isrv)) { +ioapic_eoi_broadcast (isrv); + }apic_update_irq (s); }
hw/ioapic.c +41 -2
86 lines not on this page · raw message
hw/ioapic.h new file +20 -0
@@ -0,0 +1,20 @@ +/* +* ioapic.c IOAPIC emulation logic +* +* Copyright (c) 2011 Jan Kiszka, Siemens AG +* +* This library is free software; you can redistribute it and/or +* modify it under the terms of the GNU Lesser General Public +* License as published by the Free Software Foundation; either +* version 2 of the License, or (at your option) any later version. +* +* This library is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* Lesser General Public License for more details. +* +* You should have received a copy of the GNU Lesser General Public +* License along with this library; if not, see <http://www.gnu.org/licenses/>. +*/ + +void ioapic_eoi_broadcast (int vector);
Signature · 1 line
1.7.1
Anthony Liguori
AppliedRe: [Qemu-devel] [0.14?][PATCH v3 1/4] ioapic: Implement EOI handling for level-triggered IRQs
Quoted 8 lines · Jan Kiszka
From: Jan Kiszka<[email protected]>
Add the missing EOI broadcast from local APIC to the IOAPICs on completion of level-triggered IRQs. This ensures that a still asserted IRQ source properly re-triggers an APIC IRQ.
Signed-off-by Jan Kiszka<[email protected]>
Applied to master, Thanks.
Regards,
Anthony Liguori
Quoted 165 lines
--- hw/apic.c | 9 ++++++--- hw/ioapic.c | 43 +++++++++++++++++++++++++++++++++++++++++-- hw/ioapic.h | 20 ++++++++++++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-)create mode 100644
hw/ioapic.hhw/apic.c +6
-3@@ -18,6 +18,7 @@*/ #include "hw.h" #include "apic.h" +#include "ioapic.h" #include "qemu-timer.h" #include "host-utils.h" #include "sysbus.h" @@ -57,7 +58,8 @@#define ESR_ILLEGAL_ADDRESS (1<< 7) -#define APIC_SV_ENABLE (1<< 8) +#define APIC_SV_DIRECTED_IO (1<<12) +#define APIC_SV_ENABLE (1<<8)#define MAX_APICS 255#define MAX_APIC_WORDS 8@@ -420,8 +422,9 @@ static void apic_eoi(APICState *s)if (isrv<0 )return ;reset_bit (s->isr, isrv); -/* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to -set the remote IRR bit for level triggered interrupts. */ +if (!(s->spurious_vec&APIC_SV_DIRECTED_IO )&&get_bit (s->tmr, isrv)) { +ioapic_eoi_broadcast (isrv); + }apic_update_irq (s); }hw/ioapic.c +41
-2@@ -23,6 +23,7 @@#include "hw.h" #include "pc.h" #include "apic.h" +#include "ioapic.h" #include "qemu-timer.h" #include "host-utils.h" #include "sysbus.h" @@ -36,7 +37,10 @@#define DPRINTF (fmt, ...)#endif -#define IOAPIC_LVT_MASKED (1<<16) +#define MAX_IOAPICS 1 + +#define IOAPIC_LVT_MASKED (1<< 16) +#define IOAPIC_LVT_REMOTE_IRR (1<< 14)#define IOAPIC_TRIGGER_EDGE 0#define IOAPIC_TRIGGER_LEVEL 1@@ -61,6 +65,8 @@ struct IOAPICState {uint64_t ioredtbl[IOAPIC_NUM_PINS ]; }; +static IOAPICState *ioapics[MAX_IOAPICS ]; +static void ioapic_service (IOAPICState *s) {uint8_t i;@@ -83,8 +89,11 @@ static void ioapic_service(IOAPICState *s) dest_mode = (entry>>11 )&1 ; delivery_mode = (entry>>8 )&7 ; polarity = (entry>>13 )&1 ; -if (trig_mode ==IOAPIC_TRIGGER_EDGE ) +if (trig_mode ==IOAPIC_TRIGGER_EDGE ) { s->irr&= ~mask; + }else { + s->ioredtbl[i] |=IOAPIC_LVT_REMOTE_IRR ; + }if (delivery_mode ==IOAPIC_DM_EXTINT ) vector =pic_read_irq (isa_pic);else @@ -131,6 +140,29 @@ static void ioapic_set_irq(void *opaque, int vector, int level) } } +void ioapic_eoi_broadcast (int vector) +{ +IOAPICState *s; +uint64_t entry; +int i, n; + +for (i =0 ; i<MAX_IOAPICS ; i++) { + s = ioapics[i]; +if (!s) { +continue ; + } +for (n =0 ; n<IOAPIC_NUM_PINS ; n++) { + entry = s->ioredtbl[n]; +if ((entry&IOAPIC_LVT_REMOTE_IRR )&& (entry&0xff ) == vector) { + s->ioredtbl[n] = entry& ~IOAPIC_LVT_REMOTE_IRR ; +if (!(entry&IOAPIC_LVT_MASKED )&& (s->irr& (1 << n))) { +ioapic_service (s); + } + } + } + } +} +static uint32_t ioapic_mem_readl (void *opaque,target_phys_addr_t addr) {IOAPICState *s = opaque;@@ -240,6 +272,11 @@ static int ioapic_init1(SysBusDevice *dev) {IOAPICState *s =FROM_SYSBUS (IOAPICState, dev);int io_memory; +static int ioapic_no; + +if (ioapic_no>=MAX_IOAPICS ) { +return -1 ; + } io_memory = cpu_register_io_memory(ioapic_mem_read, ioapic_mem_write, s,@@ -248,6 +285,8 @@ static int ioapic_init1(SysBusDevice *dev)qdev_init_gpio_in (&dev->qdev, ioapic_set_irq,IOAPIC_NUM_PINS ); + ioapics[ioapic_no++] = s; +return 0 ; }hw/ioapic.h new file +20
-0@@ -0,0 +1,20 @@ +/* +* ioapic.c IOAPIC emulation logic +* +* Copyright (c) 2011 Jan Kiszka, Siemens AG +* +* This library is free software; you can redistribute it and/or +* modify it under the terms of the GNU Lesser General Public +* License as published by the Free Software Foundation; either +* version 2 of the License, or (at your option) any later version. +* +* This library is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* Lesser General Public License for more details. +* +* You should have received a copy of the GNU Lesser General Public +* License along with this library; if not, see<http://www.gnu.org/licenses/>. +*/ + +void ioapic_eoi_broadcast (int vector);
[0.14?][PATCH v3 3/4] ioapic: Add support for qemu-kvm's vmstate v2
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
qemu-kvm carries the IOAPIC base address in its v2 vmstate. We only support the default base address so far, and saving even that in the device state was rejected.
Add a padding field to be able to read qemu-kvm's old state, but increase our version to 3, indicating that we are not saving a valid address. This also gives downstream the chance to change to stop evaluating the base_address and move to v3 as well.
--- hw/ioapic.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
hw/ioapic.c +2 -1
@@ -244,13 +244,14 @@ static int ioapic_post_load(void *opaque, int version_id)static const VMStateDescription vmstate_ioapic = { .name ="ioapic" , - .version_id =2 , + .version_id =3 , .post_load = ioapic_post_load, .minimum_version_id =1 , .minimum_version_id_old =1 , .fields = (VMStateField []) {VMSTATE_UINT8 (id, IOAPICState),VMSTATE_UINT8 (ioregsel, IOAPICState), +VMSTATE_UNUSED_V (2 ,8 ),/* to account for qemu-kvm's v2 format */ VMSTATE_UINT32_V (irr, IOAPICState,2 ),VMSTATE_UINT64_ARRAY (ioredtbl, IOAPICState,IOAPIC_NUM_PINS ),VMSTATE_END_OF_LIST ()
Signature · 1 line
1.7.1
[0.14?][PATCH v3 2/4] ioapic: Save/restore irr
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
This is a guest modifiable state that must be saved/restored properly.
--- hw/ioapic.c | 15 ++++++++++++++- 1 files changed, 14 insertions(+), 1 deletions(-)
hw/ioapic.c +14 -1
@@ -231,14 +231,27 @@ static void ioapic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t va } } +static int ioapic_post_load (void *opaque,int version_id) +{ +IOAPICState *s = opaque; + +if (version_id ==1 ) { +/* set sane value */ + s->irr =0 ; + } +return 0 ; +} +static const VMStateDescription vmstate_ioapic = { .name ="ioapic" , - .version_id =1 , + .version_id =2 , + .post_load = ioapic_post_load, .minimum_version_id =1 , .minimum_version_id_old =1 , .fields = (VMStateField []) {VMSTATE_UINT8 (id, IOAPICState),VMSTATE_UINT8 (ioregsel, IOAPICState), +VMSTATE_UINT32_V (irr, IOAPICState,2 ),VMSTATE_UINT64_ARRAY (ioredtbl, IOAPICState,IOAPIC_NUM_PINS ),VMSTATE_END_OF_LIST () }
Signature · 1 line
1.7.1
[0.14?][PATCH v3 4/4] ioapic: Style & magics cleanup
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Fix a few style issues and convert magic numbers into prober symbolic constants, also fixing the wrong but unused IOAPIC_DM_SIPI value.
--- hw/ioapic.c | 177 +++++++++++++++++++++++++++++++++++----------------------- 1 files changed, 107 insertions(+), 70 deletions(-)
hw/ioapic.c +107 -70
@@ -39,20 +39,48 @@#define MAX_IOAPICS 1 -#define IOAPIC_LVT_MASKED (1 << 16) -#define IOAPIC_LVT_REMOTE_IRR (1 << 14) +#define IOAPIC_VERSION 0x11 -#define IOAPIC_TRIGGER_EDGE 0 -#define IOAPIC_TRIGGER_LEVEL 1 +#define IOAPIC_LVT_DEST_SHIFT 56 +#define IOAPIC_LVT_MASKED_SHIFT 16 +#define IOAPIC_LVT_TRIGGER_MODE_SHIFT 15 +#define IOAPIC_LVT_REMOTE_IRR_SHIFT 14 +#define IOAPIC_LVT_POLARITY_SHIFT 13 +#define IOAPIC_LVT_DELIV_STATUS_SHIFT 12 +#define IOAPIC_LVT_DEST_MODE_SHIFT 11 +#define IOAPIC_LVT_DELIV_MODE_SHIFT 8 + +#define IOAPIC_LVT_MASKED (1 << IOAPIC_LVT_MASKED_SHIFT) +#define IOAPIC_LVT_REMOTE_IRR (1 << IOAPIC_LVT_REMOTE_IRR_SHIFT) + +#define IOAPIC_TRIGGER_EDGE 0 +#define IOAPIC_TRIGGER_LEVEL 1/*io{apic,sapic} delivery mode*/ -#define IOAPIC_DM_FIXED 0x0 -#define IOAPIC_DM_LOWEST_PRIORITY 0x1 -#define IOAPIC_DM_PMI 0x2 -#define IOAPIC_DM_NMI 0x4 -#define IOAPIC_DM_INIT 0x5 -#define IOAPIC_DM_SIPI 0x5 -#define IOAPIC_DM_EXTINT 0x7 +#define IOAPIC_DM_FIXED 0x0 +#define IOAPIC_DM_LOWEST_PRIORITY 0x1 +#define IOAPIC_DM_PMI 0x2 +#define IOAPIC_DM_NMI 0x4 +#define IOAPIC_DM_INIT 0x5 +#define IOAPIC_DM_SIPI 0x6 +#define IOAPIC_DM_EXTINT 0x7 +#define IOAPIC_DM_MASK 0x7 + +#define IOAPIC_VECTOR_MASK 0xff + +#define IOAPIC_IOREGSEL 0x00 +#define IOAPIC_IOWIN 0x10 + +#define IOAPIC_REG_ID 0x00 +#define IOAPIC_REG_VER 0x01 +#define IOAPIC_REG_ARB 0x02 +#define IOAPIC_REG_REDTBL_BASE 0x10 +#define IOAPIC_ID 0x00 + +#define IOAPIC_ID_SHIFT 24 +#define IOAPIC_ID_MASK 0xf + +#define IOAPIC_VER_ENTRIES_SHIFT 16typedef struct IOAPICState IOAPICState ;
@@ -60,7 +88,6 @@ struct IOAPICState {SysBusDevice busdev;uint8_t id;uint8_t ioregsel; -uint32_t irr;uint64_t ioredtbl[IOAPIC_NUM_PINS ]; };
@@ -84,21 +111,22 @@ static void ioapic_service(IOAPICState *s)if (s->irr & mask) { entry = s->ioredtbl[i];if (!(entry &IOAPIC_LVT_MASKED )) { - trig_mode = ((entry >>15 ) &1 ); - dest = entry >>56 ; - dest_mode = (entry >>11 ) &1 ; - delivery_mode = (entry >>8 ) &7 ; - polarity = (entry >>13 ) &1 ; + trig_mode = ((entry >>IOAPIC_LVT_TRIGGER_MODE_SHIFT ) &1 ); + dest = entry >>IOAPIC_LVT_DEST_SHIFT ; + dest_mode = (entry >>IOAPIC_LVT_DEST_MODE_SHIFT ) &1 ; + delivery_mode = + (entry >>IOAPIC_LVT_DELIV_MODE_SHIFT ) &IOAPIC_DM_MASK ; + polarity = (entry >>IOAPIC_LVT_POLARITY_SHIFT ) &1 ;if (trig_mode ==IOAPIC_TRIGGER_EDGE ) { s->irr &= ~mask; }else { s->ioredtbl[i] |=IOAPIC_LVT_REMOTE_IRR ; } -if (delivery_mode ==IOAPIC_DM_EXTINT ) +if (delivery_mode ==IOAPIC_DM_EXTINT ) { vector =pic_read_irq (isa_pic); -else - vector = entry &0xff ; - + }else { + vector = entry &IOAPIC_VECTOR_MASK ; + }apic_deliver_irq (dest, dest_mode, delivery_mode, vector, polarity, trig_mode); }
@@ -114,15 +142,16 @@ static void ioapic_set_irq(void *opaque, int vector, int level)* to GSI 2. GSI maps to ioapic 1-1. This is not * the cleanest way of doing it but it should work. */ -DPRINTF ("%s: %s vec %x\n" , __func__, level?"raise" :"lower" , vector); -if (vector ==0 ) +DPRINTF ("%s: %s vec %x\n" , __func__, level ?"raise" :"lower" , vector); +if (vector ==0 ) { vector =2 ; - + }if (vector >=0 && vector <IOAPIC_NUM_PINS ) {uint32_t mask =1 << vector;uint64_t entry = s->ioredtbl[vector]; -if ((entry >>15 ) &1 ) { +if (((entry >>IOAPIC_LVT_TRIGGER_MODE_SHIFT ) &1 ) == +IOAPIC_TRIGGER_LEVEL ) {/* level triggered */ if (level) { s->irr |= mask;
@@ -153,7 +182,8 @@ void ioapic_eoi_broadcast(int vector) }for (n =0 ; n <IOAPIC_NUM_PINS ; n++) { entry = s->ioredtbl[n]; -if ((entry &IOAPIC_LVT_REMOTE_IRR ) && (entry &0xff ) == vector) { +if ((entry &IOAPIC_LVT_REMOTE_IRR ) + && (entry &IOAPIC_VECTOR_MASK ) == vector) { s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR ;if (!(entry &IOAPIC_LVT_MASKED ) && (s->irr & (1 << n))) {ioapic_service (s);
@@ -169,65 +199,71 @@ static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr)int index;uint32_t val =0 ; - addr &=0xff ; -if (addr ==0x00 ) { +switch (addr &0xff ) { +case IOAPIC_IOREGSEL : val = s->ioregsel; - }else if (addr ==0x10 ) { +break ; +case IOAPIC_IOWIN :switch (s->ioregsel) { -case 0x00 : - val = s->id <<24 ; -break ; -case 0x01 : - val =0x11 | ((IOAPIC_NUM_PINS -1 ) <<16 );/* version 0x11 */ -break ; -case 0x02 : - val =0 ; -break ; -default : - index = (s->ioregsel -0x10 ) >>1 ; -if (index >=0 && index <IOAPIC_NUM_PINS ) { -if (s->ioregsel &1 ) - val = s->ioredtbl[index] >>32 ; -else - val = s->ioredtbl[index] &0xffffffff ; +case IOAPIC_REG_ID : + val = s->id <<IOAPIC_ID_SHIFT ; +break ; +case IOAPIC_REG_VER : + val =IOAPIC_VERSION | + ((IOAPIC_NUM_PINS -1 ) <<IOAPIC_VER_ENTRIES_SHIFT ); +break ; +case IOAPIC_REG_ARB : + val =0 ; +break ; +default : + index = (s->ioregsel -IOAPIC_REG_REDTBL_BASE ) >>1 ; +if (index >=0 && index <IOAPIC_NUM_PINS ) { +if (s->ioregsel &1 ) { + val = s->ioredtbl[index] >>32 ; + }else { + val = s->ioredtbl[index] &0xffffffff ; } + } }DPRINTF ("read: %08x = %08x\n" , s->ioregsel, val); +break ; }return val; } -static void ioapic_mem_writel (void *opaque,target_phys_addr_t addr,uint32_t val) +static void +ioapic_mem_writel (void *opaque,target_phys_addr_t addr,uint32_t val) {IOAPICState *s = opaque;int index; - addr &=0xff ; -if (addr ==0x00 ) { +switch (addr &0xff ) { +case IOAPIC_IOREGSEL : s->ioregsel = val; -return ; - }else if (addr ==0x10 ) { +break ; +case IOAPIC_IOWIN :DPRINTF ("write: %08x = %08x\n" , s->ioregsel, val);switch (s->ioregsel) { -case 0x00 : - s->id = (val >>24 ) &0xff ; -return ; -case 0x01 : -case 0x02 : -return ; -default : - index = (s->ioregsel -0x10 ) >>1 ; -if (index >=0 && index <IOAPIC_NUM_PINS ) { -if (s->ioregsel &1 ) { - s->ioredtbl[index] &=0xffffffff ; - s->ioredtbl[index] |= (uint64_t )val <<32 ; - }else { - s->ioredtbl[index] &= ~0xffffffffULL ; - s->ioredtbl[index] |= val; - } -ioapic_service (s); +case IOAPIC_REG_ID : + s->id = (val >>IOAPIC_ID_SHIFT ) &IOAPIC_ID_MASK ; +break ; +case IOAPIC_REG_VER : +case IOAPIC_REG_ARB : +break ; +default : + index = (s->ioregsel -IOAPIC_REG_REDTBL_BASE ) >>1 ; +if (index >=0 && index <IOAPIC_NUM_PINS ) { +if (s->ioregsel &1 ) { + s->ioredtbl[index] &=0xffffffff ; + s->ioredtbl[index] |= (uint64_t )val <<32 ; + }else { + s->ioredtbl[index] &= ~0xffffffffULL ; + s->ioredtbl[index] |= val; } +ioapic_service (s); + } } +break ; } }
@@ -248,7 +284,7 @@ static const VMStateDescription vmstate_ioapic = { .post_load = ioapic_post_load, .minimum_version_id = 1, .minimum_version_id_old = 1, - .fields = (VMStateField []) { + .fields = (VMStateField []) {VMSTATE_UINT8 (id, IOAPICState),VMSTATE_UINT8 (ioregsel, IOAPICState),VMSTATE_UNUSED_V (2 ,8 ),/* to account for qemu-kvm's v2 format */
@@ -266,8 +302,9 @@ static void ioapic_reset(DeviceState *d) s->id =0 ; s->ioregsel =0 ; s->irr =0 ; -for (i =0 ; i <IOAPIC_NUM_PINS ; i++) - s->ioredtbl[i] =1 <<16 ;/* mask LVT */ +for (i =0 ; i <IOAPIC_NUM_PINS ; i++) { + s->ioredtbl[i] =1 <<IOAPIC_LVT_MASKED_SHIFT ; + } }static CPUReadMemoryFunc *const ioapic_mem_read[3 ] = {
Signature · 1 line
1.7.1