Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756964Ab2HXHev (ORCPT ); Fri, 24 Aug 2012 03:34:51 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:65265 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1754810Ab2HXHep (ORCPT ); Fri, 24 Aug 2012 03:34:45 -0400 X-IronPort-AV: E=Sophos;i="4.77,818,1336320000"; d="scan'208";a="5714012" Message-ID: <50372FD2.3020403@cn.fujitsu.com> Date: Fri, 24 Aug 2012 15:40:02 +0800 From: Wen Congyang User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100413 Fedora/3.0.4-2.fc13 Thunderbird/3.0.4 MIME-Version: 1.0 To: Jan Kiszka CC: kvm list , qemu-devel , "linux-kernel@vger.kernel.org" , Avi Kivity , "Daniel P. Berrange" , KAMEZAWA Hiroyuki , Gleb Natapov , Blue Swirl , Eric Blake , Andrew Jones , Marcelo Tosatti Subject: Re: [PATCH v9 5/6] introduce a new qom device to deal with panicked event References: <50359561.9070001@cn.fujitsu.com> <5035962D.8050907@cn.fujitsu.com> <50360B4D.5070403@siemens.com> <503719C1.4080304@cn.fujitsu.com> <50371D85.1080601@web.de> <50372043.1020006@cn.fujitsu.com> <50371F6F.1010401@web.de> In-Reply-To: <50371F6F.1010401@web.de> X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2012/08/24 15:34:38, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2012/08/24 15:34:39, Serialize complete at 2012/08/24 15:34:39 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10302 Lines: 301 At 08/24/2012 02:30 PM, Jan Kiszka Wrote: > On 2012-08-24 08:33, Wen Congyang wrote: >> At 08/24/2012 02:21 PM, Jan Kiszka Wrote: >>> On 2012-08-24 08:05, Wen Congyang wrote: >>>> At 08/23/2012 06:51 PM, Jan Kiszka Wrote: >>>>> On 2012-08-23 04:32, Wen Congyang wrote: >>>>>> If the target is x86/x86_64, the guest's kernel will write 0x01 to the >>>>>> port KVM_PV_EVENT_PORT when it is panciked. This patch introduces a new >>>>>> qom device kvm_pv_ioport to listen this I/O port, and deal with panicked >>>>>> event according to panicked_action's value. The possible actions are: >>>>>> 1. emit QEVENT_GUEST_PANICKED only >>>>>> 2. emit QEVENT_GUEST_PANICKED and pause the guest >>>>>> 3. emit QEVENT_GUEST_PANICKED and poweroff the guest >>>>>> 4. emit QEVENT_GUEST_PANICKED and reset the guest >>>>>> >>>>>> I/O ports does not work for some targets(for example: s390). And you >>>>>> can implement another qom device, and include it's code into pv_event.c >>>>>> for such target. >>>>>> >>>>>> Note: if we emit QEVENT_GUEST_PANICKED only, and the management >>>>>> application does not receive this event(the management may not >>>>>> run when the event is emitted), the management won't know the >>>>>> guest is panicked. >>>>>> >>>>>> Signed-off-by: Wen Congyang >>>>>> --- >>>>>> hw/kvm/Makefile.objs | 2 +- >>>>>> hw/kvm/pv_event.c | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> hw/pc_piix.c | 9 +++ >>>>>> kvm.h | 2 + >>>>>> 4 files changed, 202 insertions(+), 1 deletions(-) >>>>>> create mode 100644 hw/kvm/pv_event.c >>>>>> >>>>>> diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs >>>>>> index 226497a..23e3b30 100644 >>>>>> --- a/hw/kvm/Makefile.objs >>>>>> +++ b/hw/kvm/Makefile.objs >>>>>> @@ -1 +1 @@ >>>>>> -obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o >>>>>> +obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pv_event.o >>>>>> diff --git a/hw/kvm/pv_event.c b/hw/kvm/pv_event.c >>>>>> new file mode 100644 >>>>>> index 0000000..c03dd22 >>>>>> --- /dev/null >>>>>> +++ b/hw/kvm/pv_event.c >>>>>> @@ -0,0 +1,190 @@ >>>>>> +/* >>>>>> + * QEMU KVM support, paravirtual event device >>>>>> + * >>>>>> + * Copyright Fujitsu, Corp. 2012 >>>>>> + * >>>>>> + * Authors: >>>>>> + * Wen Congyang >>>>>> + * >>>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >>>>>> + * See the COPYING file in the top-level directory. >>>>>> + * >>>>>> + */ >>>>>> + >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> +#include >>>>>> + >>>>>> +/* Possible values for action parameter. */ >>>>>> +#define PANICKED_REPORT 1 /* emit QEVENT_GUEST_PANICKED only */ >>>>>> +#define PANICKED_PAUSE 2 /* emit QEVENT_GUEST_PANICKED and pause VM */ >>>>>> +#define PANICKED_POWEROFF 3 /* emit QEVENT_GUEST_PANICKED and quit VM */ >>>>>> +#define PANICKED_RESET 4 /* emit QEVENT_GUEST_PANICKED and reset VM */ >>>>>> + >>>>>> +#define PV_EVENT_DRIVER "kvm_pv_event" >>>>>> + >>>>>> +struct PVEventAction { >>>>>> + char *panicked_action; >>>>>> + int panicked_action_value; >>>>>> +}; >>>>>> + >>>>>> +#define DEFINE_PV_EVENT_PROPERTIES(_state, _conf) \ >>>>>> + DEFINE_PROP_STRING("panicked_action", _state, _conf.panicked_action) >>>>>> + >>>>>> +static void panicked_mon_event(const char *action) >>>>>> +{ >>>>>> + QObject *data; >>>>>> + >>>>>> + data = qobject_from_jsonf("{ 'action': %s }", action); >>>>>> + monitor_protocol_event(QEVENT_GUEST_PANICKED, data); >>>>>> + qobject_decref(data); >>>>>> +} >>>>>> + >>>>>> +static void panicked_perform_action(uint32_t panicked_action) >>>>>> +{ >>>>>> + switch (panicked_action) { >>>>>> + case PANICKED_REPORT: >>>>>> + panicked_mon_event("report"); >>>>>> + break; >>>>>> + >>>>>> + case PANICKED_PAUSE: >>>>>> + panicked_mon_event("pause"); >>>>>> + vm_stop(RUN_STATE_GUEST_PANICKED); >>>>>> + break; >>>>>> + >>>>>> + case PANICKED_POWEROFF: >>>>>> + panicked_mon_event("poweroff"); >>>>>> + qemu_system_shutdown_request(); >>>>>> + break; >>>>>> + >>>>>> + case PANICKED_RESET: >>>>>> + panicked_mon_event("reset"); >>>>>> + qemu_system_reset_request(); >>>>>> + break; >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +static uint64_t supported_event(void) >>>>>> +{ >>>>>> + return 1 << KVM_PV_FEATURE_PANICKED; >>>>>> +} >>>>>> + >>>>>> +static void handle_event(int event, struct PVEventAction *conf) >>>>>> +{ >>>>>> + if (event == KVM_PV_EVENT_PANICKED) { >>>>>> + panicked_perform_action(conf->panicked_action_value); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +static int pv_event_init(struct PVEventAction *conf) >>>>>> +{ >>>>>> + if (!conf->panicked_action) { >>>>>> + conf->panicked_action_value = PANICKED_REPORT; >>>>>> + } else if (strcasecmp(conf->panicked_action, "none") == 0) { >>>>>> + conf->panicked_action_value = PANICKED_REPORT; >>>>>> + } else if (strcasecmp(conf->panicked_action, "pause") == 0) { >>>>>> + conf->panicked_action_value = PANICKED_PAUSE; >>>>>> + } else if (strcasecmp(conf->panicked_action, "poweroff") == 0) { >>>>>> + conf->panicked_action_value = PANICKED_POWEROFF; >>>>>> + } else if (strcasecmp(conf->panicked_action, "reset") == 0) { >>>>>> + conf->panicked_action_value = PANICKED_RESET; >>>>>> + } else { >>>>>> + return -1; >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +#if defined(KVM_PV_EVENT_PORT) >>>>>> + >>>>>> +#include "hw/isa.h" >>>>>> + >>>>>> +typedef struct { >>>>>> + ISADevice dev; >>>>>> + struct PVEventAction conf; >>>>>> + MemoryRegion ioport; >>>>>> +} PVIOPortState; >>>>>> + >>>>>> +static uint64_t pv_io_read(void *opaque, target_phys_addr_t addr, unsigned size) >>>>>> +{ >>>>>> + return supported_event(); >>>>>> +} >>>>>> + >>>>>> +static void pv_io_write(void *opaque, target_phys_addr_t addr, uint64_t val, >>>>>> + unsigned size) >>>>>> +{ >>>>>> + PVIOPortState *s = opaque; >>>>>> + >>>>>> + handle_event(val, &s->conf); >>>>>> +} >>>>>> + >>>>>> +static const MemoryRegionOps pv_io_ops = { >>>>>> + .read = pv_io_read, >>>>>> + .write = pv_io_write, >>>>>> + .impl = { >>>>>> + .min_access_size = 4, >>>>>> + .max_access_size = 4, >>>>>> + }, >>>>>> +}; >>>>>> + >>>>>> +static int pv_ioport_initfn(ISADevice *dev) >>>>>> +{ >>>>>> + PVIOPortState *s = DO_UPCAST(PVIOPortState, dev, dev); >>>>>> + >>>>>> + if (pv_event_init(&s->conf) < 0) { >>>>>> + return -1; >>>>>> + } >>>>>> + >>>>>> + memory_region_init_io(&s->ioport, &pv_io_ops, s, "pv_event", 1); >>>>>> + isa_register_ioport(dev, &s->ioport, KVM_PV_EVENT_PORT); >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static Property pv_ioport_properties[] = { >>>>>> + DEFINE_PV_EVENT_PROPERTIES(PVIOPortState, conf), >>>>>> + DEFINE_PROP_END_OF_LIST(), >>>>>> +}; >>>>>> + >>>>>> +static void pv_ioport_class_init(ObjectClass *klass, void *data) >>>>>> +{ >>>>>> + DeviceClass *dc = DEVICE_CLASS(klass); >>>>>> + ISADeviceClass *ic = ISA_DEVICE_CLASS(klass); >>>>>> + >>>>>> + ic->init = pv_ioport_initfn; >>>>>> + dc->no_user = 1; >>>>>> + dc->props = pv_ioport_properties; >>>>>> +} >>>>>> + >>>>>> +static TypeInfo pv_ioport_info = { >>>>>> + .name = PV_EVENT_DRIVER, >>>>>> + .parent = TYPE_ISA_DEVICE, >>>>>> + .instance_size = sizeof(PVIOPortState), >>>>>> + .class_init = pv_ioport_class_init, >>>>>> +}; >>>>>> + >>>>>> +static void pv_ioport_register_types(void) >>>>>> +{ >>>>>> + type_register_static(&pv_ioport_info); >>>>>> +} >>>>>> + >>>>>> +type_init(pv_ioport_register_types) >>>>>> + >>>>>> +void kvm_pv_event_init(void *opaque) >>>>>> +{ >>>>>> + ISABus *bus = opaque; >>>>>> + ISADevice *dev; >>>>>> + >>>>>> + dev = isa_create(bus, PV_EVENT_DRIVER); >>>>>> + qdev_init_nofail(&dev->qdev); >>>>>> +} >>>>>> + >>>>>> +#else >>>>>> +void kvm_pv_event_init(void *opaque) >>>>>> +{ >>>>> >>>>> Some comment that this stub requires an implementation whenever it is >>>>> actually built would be helpful. Something that explains a different >>>>> transport than PIO will be needed. >>>> >>>> OK >>>> >>>>> >>>>>> +} >>>>>> +#endif >>>>>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c >>>>>> index 88ff041..f73fb85 100644 >>>>>> --- a/hw/pc_piix.c >>>>>> +++ b/hw/pc_piix.c >>>>>> @@ -46,6 +46,9 @@ >>>>>> #ifdef CONFIG_XEN >>>>>> # include >>>>>> #endif >>>>>> +#ifdef CONFIG_KVM >>>>>> +# include >>>>>> +#endif >>>>>> >>>>>> #define MAX_IDE_BUS 2 >>>>>> >>>>>> @@ -285,6 +288,12 @@ static void pc_init1(MemoryRegion *system_memory, >>>>>> if (pci_enabled) { >>>>>> pc_pci_device_init(pci_bus); >>>>>> } >>>>>> + >>>>>> +#ifdef KVM_PV_EVENT_PORT >>>>> >>>>> This file is x86-only, and there we have KVM_PV_EVENT_PORT >>>>> unconditionally. So drop the #ifdef. >>>>> >>>>>> + if (kvm_enabled()) { >>>>>> + kvm_pv_event_init(isa_bus); >>>>> >>>>> But you are missing a kvm-stub entry for kvm_pv_event_init. A >>>>> --disable-kvm build should be broken for that reason. >>>> >>>> Hmm, KVM_PV_EVENT_PORT is defined in asm/kvm_para.h, and I include >>>> this file only when CONFIG_KVM is defined. So --disable-kvm build >>>> does not be broken in my test. >>> >>> Yeah, but that is a bit ugly and another reason to go for a proper kvm-stub. >> >> Yes, it is ugly. I will add a stub function in kvm-stub. And the header >> file asm/kvm_para.h can be included unconditionally. And "#ifdef KVM_PV_EVENT_PORT" >> can be dropped. > > Actually, not asm/kvm_para.h but linux/kvm_para.h. And you will then > only need it in pv_event.c. OK. Thanks Wen Congyang > > Jan > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/