| Message ID | 1470059959-372-1-git-send-email-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 1 Aug 2016 21:59:19 +0800 Peter Xu <peterx@redhat.com> wrote: > Some old Linux kernels (upstream before v4.0), or any released RHEL > kernels has problem in sending APIC EOI when IR is enabled. Meanwhile, > many of them only support explicit EOI for IOAPIC, which is only > introduced in IOAPIC version 0x20. This patch provide a way to boost s/provide/provides/ > QEMU IOAPIC to version 0x20, in order for QEMU to correctly receive EOI > messages. > > Without boosting IOAPIC version to 0x20, kernels before commit d32932d > ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces") > will have trouble enabling both IR and level-triggered interrupt devices > (like e1000). > > To upgrade IOAPIC to version 0x20, we need to specify: > > -global ioapic.version=0x20 > > To be compatible with old systems, 0x11 will still be the default IOAPIC > version. Here 0x11 and 0x20 are the only versions to be supported. Is there a reason not to default to 0x20 for new machine types and set 2.6 and older machine types to 0x11? > > One thing to mention: this patch only applies to emulated IOAPIC. It > does not affect kernel IOAPIC behavior. ^^^ which is .... ? > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > hw/intc/ioapic.c | 22 +++++++++++++++++++++- > include/hw/i386/ioapic_internal.h | 4 ++-- > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c > index 2d3282a..e8568d2 100644 > --- a/hw/intc/ioapic.c > +++ b/hw/intc/ioapic.c > @@ -21,6 +21,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "monitor/monitor.h" > #include "hw/hw.h" > #include "hw/i386/pc.h" > @@ -265,7 +266,7 @@ ioapic_mem_read(void *opaque, hwaddr addr, unsigned int size) > val = s->id << IOAPIC_ID_SHIFT; > break; > case IOAPIC_REG_VER: > - val = IOAPIC_VERSION | > + val = s->version | > ((IOAPIC_NUM_PINS - 1) << IOAPIC_VER_ENTRIES_SHIFT); > break; > default: > @@ -354,6 +355,13 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val, > } > } > break; > + case IOAPIC_EOI: > + /* Explicit EOI is only supported for IOAPIC version 0x20 */ > + if (size != 4 || s->version != 0x20) { > + break; > + } > + ioapic_eoi_broadcast(val); > + break; > } > > ioapic_update_kvm_routes(s); > @@ -387,6 +395,12 @@ static void ioapic_realize(DeviceState *dev, Error **errp) > { > IOAPICCommonState *s = IOAPIC_COMMON(dev); > > + if (s->version != 0x11 && s->version != 0x20) { > + error_report("IOAPIC only supports version 0x11 or 0x20 " > + "(default: 0x11)."); probably no need to say what's default here. > + exit(1); For erro handling realize() typically calls error_setg() + error_propagate() instead of directly error_report() + exit() It should work in this case as well as the caller ioapic_init_gsi()->qdev_init_nofail() will terminate QEMU is "realize" = true fails. > + } > + > memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s, > "ioapic", 0x1000); > > @@ -397,6 +411,11 @@ static void ioapic_realize(DeviceState *dev, Error **errp) > qemu_add_machine_init_done_notifier(&s->machine_done); > } > > +static Property ioapic_properties[] = { > + DEFINE_PROP_UINT8("version", IOAPICCommonState, version, 0x11), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void ioapic_class_init(ObjectClass *klass, void *data) > { > IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass); > @@ -404,6 +423,7 @@ static void ioapic_class_init(ObjectClass *klass, void *data) > > k->realize = ioapic_realize; > dc->reset = ioapic_reset_common; > + dc->props = ioapic_properties; > } > > static const TypeInfo ioapic_info = { > diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h > index d89ea1b..a11d86d 100644 > --- a/include/hw/i386/ioapic_internal.h > +++ b/include/hw/i386/ioapic_internal.h > @@ -29,8 +29,6 @@ > > #define MAX_IOAPICS 1 > > -#define IOAPIC_VERSION 0x11 > - > #define IOAPIC_LVT_DEST_SHIFT 56 > #define IOAPIC_LVT_DEST_IDX_SHIFT 48 > #define IOAPIC_LVT_MASKED_SHIFT 16 > @@ -71,6 +69,7 @@ > > #define IOAPIC_IOREGSEL 0x00 > #define IOAPIC_IOWIN 0x10 > +#define IOAPIC_EOI 0x40 > > #define IOAPIC_REG_ID 0x00 > #define IOAPIC_REG_VER 0x01 > @@ -109,6 +108,7 @@ struct IOAPICCommonState { > uint32_t irr; > uint64_t ioredtbl[IOAPIC_NUM_PINS]; > Notifier machine_done; > + uint8_t version; > }; > > void ioapic_reset_common(DeviceState *dev);
On 01/08/2016 16:43, Igor Mammedov wrote: > > To be compatible with old systems, 0x11 will still be the default IOAPIC > > version. Here 0x11 and 0x20 are the only versions to be supported. > > Is there a reason not to default to 0x20 for new machine types and > set 2.6 and older machine types to 0x11? See discussion in v2. It's just for safety, what you just said is indeed the plan for 2.8. This patch already provides the necessary infrastructure. Paolo
On 01/08/2016 15:59, Peter Xu wrote: > Some old Linux kernels (upstream before v4.0), or any released RHEL > kernels has problem in sending APIC EOI when IR is enabled. Meanwhile, > many of them only support explicit EOI for IOAPIC, which is only > introduced in IOAPIC version 0x20. This patch provide a way to boost > QEMU IOAPIC to version 0x20, in order for QEMU to correctly receive EOI > messages. > > Without boosting IOAPIC version to 0x20, kernels before commit d32932d > ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces") > will have trouble enabling both IR and level-triggered interrupt devices > (like e1000). > > To upgrade IOAPIC to version 0x20, we need to specify: > > -global ioapic.version=0x20 > > To be compatible with old systems, 0x11 will still be the default IOAPIC > version. Here 0x11 and 0x20 are the only versions to be supported. > > One thing to mention: this patch only applies to emulated IOAPIC. It > does not affect kernel IOAPIC behavior. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > hw/intc/ioapic.c | 22 +++++++++++++++++++++- > include/hw/i386/ioapic_internal.h | 4 ++-- > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c > index 2d3282a..e8568d2 100644 > --- a/hw/intc/ioapic.c > +++ b/hw/intc/ioapic.c > @@ -21,6 +21,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "monitor/monitor.h" > #include "hw/hw.h" > #include "hw/i386/pc.h" > @@ -265,7 +266,7 @@ ioapic_mem_read(void *opaque, hwaddr addr, unsigned int size) > val = s->id << IOAPIC_ID_SHIFT; > break; > case IOAPIC_REG_VER: > - val = IOAPIC_VERSION | > + val = s->version | > ((IOAPIC_NUM_PINS - 1) << IOAPIC_VER_ENTRIES_SHIFT); > break; > default: > @@ -354,6 +355,13 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val, > } > } > break; > + case IOAPIC_EOI: > + /* Explicit EOI is only supported for IOAPIC version 0x20 */ > + if (size != 4 || s->version != 0x20) { > + break; > + } > + ioapic_eoi_broadcast(val); > + break; > } > > ioapic_update_kvm_routes(s); > @@ -387,6 +395,12 @@ static void ioapic_realize(DeviceState *dev, Error **errp) > { > IOAPICCommonState *s = IOAPIC_COMMON(dev); > > + if (s->version != 0x11 && s->version != 0x20) { > + error_report("IOAPIC only supports version 0x11 or 0x20 " > + "(default: 0x11)."); > + exit(1); > + } > + > memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s, > "ioapic", 0x1000); > > @@ -397,6 +411,11 @@ static void ioapic_realize(DeviceState *dev, Error **errp) > qemu_add_machine_init_done_notifier(&s->machine_done); > } > > +static Property ioapic_properties[] = { > + DEFINE_PROP_UINT8("version", IOAPICCommonState, version, 0x11), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void ioapic_class_init(ObjectClass *klass, void *data) > { > IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass); > @@ -404,6 +423,7 @@ static void ioapic_class_init(ObjectClass *klass, void *data) > > k->realize = ioapic_realize; > dc->reset = ioapic_reset_common; > + dc->props = ioapic_properties; > } > > static const TypeInfo ioapic_info = { > diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h > index d89ea1b..a11d86d 100644 > --- a/include/hw/i386/ioapic_internal.h > +++ b/include/hw/i386/ioapic_internal.h > @@ -29,8 +29,6 @@ > > #define MAX_IOAPICS 1 > > -#define IOAPIC_VERSION 0x11 > - > #define IOAPIC_LVT_DEST_SHIFT 56 > #define IOAPIC_LVT_DEST_IDX_SHIFT 48 > #define IOAPIC_LVT_MASKED_SHIFT 16 > @@ -71,6 +69,7 @@ > > #define IOAPIC_IOREGSEL 0x00 > #define IOAPIC_IOWIN 0x10 > +#define IOAPIC_EOI 0x40 > > #define IOAPIC_REG_ID 0x00 > #define IOAPIC_REG_VER 0x01 > @@ -109,6 +108,7 @@ struct IOAPICCommonState { > uint32_t irr; > uint64_t ioredtbl[IOAPIC_NUM_PINS]; > Notifier machine_done; > + uint8_t version; > }; > > void ioapic_reset_common(DeviceState *dev); > Thanks, queued for 2.7. Paolo
On Mon, Aug 01, 2016 at 04:43:26PM +0200, Igor Mammedov wrote: > > One thing to mention: this patch only applies to emulated IOAPIC. It > > does not affect kernel IOAPIC behavior. > ^^^ which is .... ? What I wanted to express is that this property will not taking effect when kernel ioapic is used (so kernel IOAPIC will still have version 0x11). However after a second thought I think this sentence can be removed as well, since we are using "kvm-ioapic" device for kernel IOAPIC, and property name "ioapic.version" should explain itself already. :) Thanks for review, -- peterx
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c index 2d3282a..e8568d2 100644 --- a/hw/intc/ioapic.c +++ b/hw/intc/ioapic.c @@ -21,6 +21,7 @@ */ #include "qemu/osdep.h" +#include "qemu/error-report.h" #include "monitor/monitor.h" #include "hw/hw.h" #include "hw/i386/pc.h" @@ -265,7 +266,7 @@ ioapic_mem_read(void *opaque, hwaddr addr, unsigned int size) val = s->id << IOAPIC_ID_SHIFT; break; case IOAPIC_REG_VER: - val = IOAPIC_VERSION | + val = s->version | ((IOAPIC_NUM_PINS - 1) << IOAPIC_VER_ENTRIES_SHIFT); break; default: @@ -354,6 +355,13 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val, } } break; + case IOAPIC_EOI: + /* Explicit EOI is only supported for IOAPIC version 0x20 */ + if (size != 4 || s->version != 0x20) { + break; + } + ioapic_eoi_broadcast(val); + break; } ioapic_update_kvm_routes(s); @@ -387,6 +395,12 @@ static void ioapic_realize(DeviceState *dev, Error **errp) { IOAPICCommonState *s = IOAPIC_COMMON(dev); + if (s->version != 0x11 && s->version != 0x20) { + error_report("IOAPIC only supports version 0x11 or 0x20 " + "(default: 0x11)."); + exit(1); + } + memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s, "ioapic", 0x1000); @@ -397,6 +411,11 @@ static void ioapic_realize(DeviceState *dev, Error **errp) qemu_add_machine_init_done_notifier(&s->machine_done); } +static Property ioapic_properties[] = { + DEFINE_PROP_UINT8("version", IOAPICCommonState, version, 0x11), + DEFINE_PROP_END_OF_LIST(), +}; + static void ioapic_class_init(ObjectClass *klass, void *data) { IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass); @@ -404,6 +423,7 @@ static void ioapic_class_init(ObjectClass *klass, void *data) k->realize = ioapic_realize; dc->reset = ioapic_reset_common; + dc->props = ioapic_properties; } static const TypeInfo ioapic_info = { diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h index d89ea1b..a11d86d 100644 --- a/include/hw/i386/ioapic_internal.h +++ b/include/hw/i386/ioapic_internal.h @@ -29,8 +29,6 @@ #define MAX_IOAPICS 1 -#define IOAPIC_VERSION 0x11 - #define IOAPIC_LVT_DEST_SHIFT 56 #define IOAPIC_LVT_DEST_IDX_SHIFT 48 #define IOAPIC_LVT_MASKED_SHIFT 16 @@ -71,6 +69,7 @@ #define IOAPIC_IOREGSEL 0x00 #define IOAPIC_IOWIN 0x10 +#define IOAPIC_EOI 0x40 #define IOAPIC_REG_ID 0x00 #define IOAPIC_REG_VER 0x01 @@ -109,6 +108,7 @@ struct IOAPICCommonState { uint32_t irr; uint64_t ioredtbl[IOAPIC_NUM_PINS]; Notifier machine_done; + uint8_t version; }; void ioapic_reset_common(DeviceState *dev);
Some old Linux kernels (upstream before v4.0), or any released RHEL kernels has problem in sending APIC EOI when IR is enabled. Meanwhile, many of them only support explicit EOI for IOAPIC, which is only introduced in IOAPIC version 0x20. This patch provide a way to boost QEMU IOAPIC to version 0x20, in order for QEMU to correctly receive EOI messages. Without boosting IOAPIC version to 0x20, kernels before commit d32932d ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces") will have trouble enabling both IR and level-triggered interrupt devices (like e1000). To upgrade IOAPIC to version 0x20, we need to specify: -global ioapic.version=0x20 To be compatible with old systems, 0x11 will still be the default IOAPIC version. Here 0x11 and 0x20 are the only versions to be supported. One thing to mention: this patch only applies to emulated IOAPIC. It does not affect kernel IOAPIC behavior. Signed-off-by: Peter Xu <peterx@redhat.com> --- hw/intc/ioapic.c | 22 +++++++++++++++++++++- include/hw/i386/ioapic_internal.h | 4 ++-- 2 files changed, 23 insertions(+), 3 deletions(-)