Hi,
On Wed, Apr 10, 2013 at 01:54:04PM +0200, Markus Armbruster wrote:
> Hu Tao <hu...@cn.fujitsu.com> writes:
>
> > pvpanic device is used to send guest panic event from guest to qemu.
> >
> > When guest panic happens, pvpanic device driver will write a event
> > number to IO port 0x505(which is the IO port occupied by pvpanic device,
> > by default). On receiving the event, pvpanic device will pause guest
> > cpu(s), and send a qmp event QEVENT_GUEST_PANICKED.
> >
> > Signed-off-by: Wen Congyang <we...@cn.fujitsu.com>
> > Signed-off-by: Hu Tao <hu...@cn.fujitsu.com>
> > ---
> > hw/misc/Makefile.objs | 2 +
> > hw/misc/pvpanic.c | 116
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 118 insertions(+)
> > create mode 100644 hw/misc/pvpanic.c
> >
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index 03699c3..d72ea83 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -38,3 +38,5 @@ obj-$(CONFIG_OMAP) += omap_tap.o
> > obj-$(CONFIG_PXA2XX) += pxa2xx_pcmcia.o
> > obj-$(CONFIG_SLAVIO) += slavio_misc.o
> > obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> > +
> > +common-obj-y += pvpanic.o
> > diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
> > new file mode 100644
> > index 0000000..5118fd7
> > --- /dev/null
> > +++ b/hw/misc/pvpanic.c
> > @@ -0,0 +1,116 @@
> > +/*
> > + * QEMU simulated pvpanic device.
> > + *
> > + * Copyright Fujitsu, Corp. 2013
> > + *
> > + * Authors:
> > + * Wen Congyang <we...@cn.fujitsu.com>
> > + * Hu Tao <hu...@cn.fujitsu.com>
> > + *
> > + * 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 <qapi/qmp/qobject.h>
> > +#include <qapi/qmp/qjson.h>
> > +#include <monitor/monitor.h>
> > +#include <sysemu/sysemu.h>
> > +#include <sysemu/kvm.h>
> > +
> > +/* The bit of supported pv event */
> > +#define PVPANIC_F_PANICKED 0
> > +
> > +/* The pv event value */
> > +#define PVPANIC_PANICKED (1 << PVPANIC_F_PANICKED)
> > +
> > +#define TYPE_ISA_PVPANIC_DEVICE "pvpanic"
> > +#define ISA_PVPANIC_DEVICE(obj) \
> > + OBJECT_CHECK(PVPanicState, (obj), TYPE_ISA_PVPANIC_DEVICE)
> > +
> > +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 handle_event(int event)
> > +{
> > + if (event == PVPANIC_PANICKED) {
> > + panicked_mon_event("pause");
> > + vm_stop(RUN_STATE_GUEST_PANICKED);
> > + return;
> > + }
> > +}
>
> I've asked these questions before, if you answered them, I missed it:
Sorry, I must have missed them.
>
> 1. Your event value looks like it encodes multiple events as bits. Only
> one bit is defined so far (PVEVENT_F_PANICKED). But you recognize this
It was the intention to support multiple events, but Gleb suggested to do
only panic event. So pvpanic device supports only panic event.
> bit only when the other bits are all off. Why? Won't we regret this if
> we ever want to define additional bits?
Other bits are reserved now, and must be written as 0. (see patch 5/7)
If we define these bits in the further for whatever purposes, we have to
update code here.
>
> 2. You silently ignore unrecognized event values. Shouldn't we log
> them?
See above.
--
Regards,
Hu Tao
