From f5e0a8f42fbc5c7c2b8c0720ee657aba6cc122fd Mon Sep 17 00:00:00 2001 From: Philippe Mathieu-Daudé Date: Sat, 27 Apr 2019 16:40:23 +0200 Subject: hw/acpi/piix4: Move TYPE_PIIX4_PM to a public header Move the TYPE_PIIX4_PM definition to the corresponding header, so other files can use it. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20190427144025.22880-2-philmd@redhat.com> Signed-off-by: Paolo Bonzini --- include/hw/acpi/piix4.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h index 26c2370e30..57d7e1cda2 100644 --- a/include/hw/acpi/piix4.h +++ b/include/hw/acpi/piix4.h @@ -1,6 +1,8 @@ #ifndef HW_ACPI_PIIX4_H #define HW_ACPI_PIIX4_H +#define TYPE_PIIX4_PM "PIIX4_PM" + Object *piix4_pm_find(void); #endif -- cgit v1.2.3-55-g7522 From 81c48dd79655296f5bf94823e8ac95902a8ac3e4 Mon Sep 17 00:00:00 2001 From: Philippe Mathieu-Daudé Date: Sat, 27 Apr 2019 16:40:24 +0200 Subject: hw/i386/acpi: Add object_resolve_type_unambiguous to improve modularity When building with CONFIG_Q35=n, we get: LINK x86_64-softmmu/qemu-system-x86_64 /usr/bin/ld: hw/i386/acpi-build.o: in function `acpi_get_misc_info': /source/qemu/hw/i386/acpi-build.c:243: undefined reference to `ich9_lpc_find' collect2: error: ld returned 1 exit status make[1]: *** [Makefile:204: qemu-system-x86_64] Error 1 This is due to a dependency in acpi-build.c on the ICH9_LPC (via ich9_lpc_find) and PIIX4_PM (via piix4_pm_find) devices. To allow better modularity (compile acpi-build.c with only Q35/ICH9 or ISAPC/PIIX4), refactor the similar helper as object_resolve_type_unambiguous(). This way we relax the linker dependencies and can build the x86 targets with a selection of machines (instead of all of them). Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20190427144025.22880-3-philmd@redhat.com> Signed-off-by: Paolo Bonzini --- hw/acpi/piix4.c | 11 ----------- hw/i386/acpi-build.c | 20 ++++++++++++++++---- hw/isa/lpc_ich9.c | 11 ----------- include/hw/acpi/piix4.h | 2 -- include/hw/i386/ich9.h | 2 -- 5 files changed, 16 insertions(+), 30 deletions(-) (limited to 'include') diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 160e7308c5..c903e65169 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -552,17 +552,6 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp) piix4_pm_add_propeties(s); } -Object *piix4_pm_find(void) -{ - bool ambig; - Object *o = object_resolve_path_type("", TYPE_PIIX4_PM, &ambig); - - if (ambig || !o) { - return NULL; - } - return o; -} - I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base, qemu_irq sci_irq, qemu_irq smi_irq, int smm_enabled, DeviceState **piix4_pm) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 416da318ae..123ff2b816 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -35,6 +35,7 @@ #include "hw/acpi/acpi-defs.h" #include "hw/acpi/acpi.h" #include "hw/acpi/cpu.h" +#include "hw/acpi/piix4.h" #include "hw/nvram/fw_cfg.h" #include "hw/acpi/bios-linker-loader.h" #include "hw/loader.h" @@ -164,10 +165,21 @@ static void init_common_fadt_data(Object *o, AcpiFadtData *data) *data = fadt; } +static Object *object_resolve_type_unambiguous(const char *typename) +{ + bool ambig; + Object *o = object_resolve_path_type("", typename, &ambig); + + if (ambig || !o) { + return NULL; + } + return o; +} + static void acpi_get_pm_info(AcpiPmInfo *pm) { - Object *piix = piix4_pm_find(); - Object *lpc = ich9_lpc_find(); + Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); + Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); Object *obj = piix ? piix : lpc; QObject *o; pm->cpu_hp_io_base = 0; @@ -228,8 +240,8 @@ static void acpi_get_pm_info(AcpiPmInfo *pm) static void acpi_get_misc_info(AcpiMiscInfo *info) { - Object *piix = piix4_pm_find(); - Object *lpc = ich9_lpc_find(); + Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM); + Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE); assert(!!piix != !!lpc); if (piix) { diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index ac44aa53be..031ee9cd93 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -624,17 +624,6 @@ static const MemoryRegionOps ich9_rst_cnt_ops = { .endianness = DEVICE_LITTLE_ENDIAN }; -Object *ich9_lpc_find(void) -{ - bool ambig; - Object *o = object_resolve_path_type("", TYPE_ICH9_LPC_DEVICE, &ambig); - - if (ambig) { - return NULL; - } - return o; -} - static void ich9_lpc_get_sci_int(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h index 57d7e1cda2..028bb53e3d 100644 --- a/include/hw/acpi/piix4.h +++ b/include/hw/acpi/piix4.h @@ -3,6 +3,4 @@ #define TYPE_PIIX4_PM "PIIX4_PM" -Object *piix4_pm_find(void); - #endif diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h index 673d13d28f..046bcf33be 100644 --- a/include/hw/i386/ich9.h +++ b/include/hw/i386/ich9.h @@ -81,8 +81,6 @@ typedef struct ICH9LPCState { qemu_irq gsi[GSI_NUM_PINS]; } ICH9LPCState; -Object *ich9_lpc_find(void); - #define Q35_MASK(bit, ms_bit, ls_bit) \ ((uint##bit##_t)(((1ULL << ((ms_bit) + 1)) - 1) & ~((1ULL << ls_bit) - 1))) -- cgit v1.2.3-55-g7522 From 958a01dab8e02fc49f4fd619fad8c82a1108afdb Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Tue, 2 Apr 2019 10:02:15 +0200 Subject: ioapic: allow buggy guests mishandling level-triggered interrupts to make progress It was found that Hyper-V 2016 on KVM in some configurations (q35 machine + piix4-usb-uhci) hangs on boot. Root-cause was that one of Hyper-V level-triggered interrupt handler performs EOI before fixing the cause of the interrupt. This results in IOAPIC keep re-raising the level-triggered interrupt after EOI because irq-line remains asserted. Gory details: https://www.spinics.net/lists/kvm/msg184484.html (the whole thread). Turns out we were dealing with similar issues before; in-kernel IOAPIC implementation has commit 184564efae4d ("kvm: ioapic: conditionally delay irq delivery duringeoi broadcast") which describes a very similar issue. Steal the idea from the above mentioned commit for IOAPIC implementation in QEMU. SUCCESSIVE_IRQ_MAX_COUNT, delay and the comment are borrowed as well. Signed-off-by: Vitaly Kuznetsov Message-Id: <20190402080215.10747-1-vkuznets@redhat.com> Signed-off-by: Paolo Bonzini --- hw/intc/ioapic.c | 57 +++++++++++++++++++++++++++++++++++---- hw/intc/trace-events | 1 + include/hw/i386/ioapic_internal.h | 3 +++ 3 files changed, 56 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c index 9d75f84d3b..7074489fdf 100644 --- a/hw/intc/ioapic.c +++ b/hw/intc/ioapic.c @@ -139,6 +139,15 @@ static void ioapic_service(IOAPICCommonState *s) } } +#define SUCCESSIVE_IRQ_MAX_COUNT 10000 + +static void delayed_ioapic_service_cb(void *opaque) +{ + IOAPICCommonState *s = opaque; + + ioapic_service(s); +} + static void ioapic_set_irq(void *opaque, int vector, int level) { IOAPICCommonState *s = opaque; @@ -222,13 +231,39 @@ 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 & IOAPIC_VECTOR_MASK) == vector) { - trace_ioapic_clear_remote_irr(n, vector); - s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR; - if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) { + + if ((entry & IOAPIC_VECTOR_MASK) != vector || + ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) != IOAPIC_TRIGGER_LEVEL) { + continue; + } + + if (!(entry & IOAPIC_LVT_REMOTE_IRR)) { + continue; + } + + trace_ioapic_clear_remote_irr(n, vector); + s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR; + + if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) { + ++s->irq_eoi[vector]; + if (s->irq_eoi[vector] >= SUCCESSIVE_IRQ_MAX_COUNT) { + /* + * Real hardware does not deliver the interrupt immediately + * during eoi broadcast, and this lets a buggy guest make + * slow progress even if it does not correctly handle a + * level-triggered interrupt. Emulate this behavior if we + * detect an interrupt storm. + */ + s->irq_eoi[vector] = 0; + timer_mod_anticipate(s->delayed_ioapic_service_timer, + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + + NANOSECONDS_PER_SECOND / 100); + trace_ioapic_eoi_delayed_reassert(vector); + } else { ioapic_service(s); } + } else { + s->irq_eoi[vector] = 0; } } } @@ -401,6 +436,9 @@ static void ioapic_realize(DeviceState *dev, Error **errp) memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s, "ioapic", 0x1000); + s->delayed_ioapic_service_timer = + timer_new_ns(QEMU_CLOCK_VIRTUAL, delayed_ioapic_service_cb, s); + qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS); ioapics[ioapic_no] = s; @@ -408,6 +446,14 @@ static void ioapic_realize(DeviceState *dev, Error **errp) qemu_add_machine_init_done_notifier(&s->machine_done); } +static void ioapic_unrealize(DeviceState *dev, Error **errp) +{ + IOAPICCommonState *s = IOAPIC_COMMON(dev); + + timer_del(s->delayed_ioapic_service_timer); + timer_free(s->delayed_ioapic_service_timer); +} + static Property ioapic_properties[] = { DEFINE_PROP_UINT8("version", IOAPICCommonState, version, IOAPIC_VER_DEF), DEFINE_PROP_END_OF_LIST(), @@ -419,6 +465,7 @@ static void ioapic_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); k->realize = ioapic_realize; + k->unrealize = ioapic_unrealize; /* * If APIC is in kernel, we need to update the kernel cache after * migration, otherwise first 24 gsi routes will be invalid. diff --git a/hw/intc/trace-events b/hw/intc/trace-events index a28bdce925..90c9d07c1a 100644 --- a/hw/intc/trace-events +++ b/hw/intc/trace-events @@ -25,6 +25,7 @@ apic_mem_writel(uint64_t addr, uint32_t val) "0x%"PRIx64" = 0x%08x" ioapic_set_remote_irr(int n) "set remote irr for pin %d" ioapic_clear_remote_irr(int n, int vector) "clear remote irr for pin %d vector %d" ioapic_eoi_broadcast(int vector) "EOI broadcast for vector %d" +ioapic_eoi_delayed_reassert(int vector) "delayed reassert on EOI broadcast for vector %d" ioapic_mem_read(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem read addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" retval 0x%"PRIx32 ioapic_mem_write(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem write addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" val 0x%"PRIx32 ioapic_set_irq(int vector, int level) "vector: %d level: %d" diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h index 9848f391bb..07002f9662 100644 --- a/include/hw/i386/ioapic_internal.h +++ b/include/hw/i386/ioapic_internal.h @@ -96,6 +96,7 @@ typedef struct IOAPICCommonClass { SysBusDeviceClass parent_class; DeviceRealize realize; + DeviceUnrealize unrealize; void (*pre_save)(IOAPICCommonState *s); void (*post_load)(IOAPICCommonState *s); } IOAPICCommonClass; @@ -111,6 +112,8 @@ struct IOAPICCommonState { uint8_t version; uint64_t irq_count[IOAPIC_NUM_PINS]; int irq_level[IOAPIC_NUM_PINS]; + int irq_eoi[IOAPIC_NUM_PINS]; + QEMUTimer *delayed_ioapic_service_timer; }; void ioapic_reset_common(DeviceState *dev); -- cgit v1.2.3-55-g7522