[Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[H. Peter Anvin]

Add generic support for debugging consoles (simple I/O ports which
when written to cause debugging output to be written to a target.)
The current implementation matches Bochs' port 0xe9, allowing the same
debugging code to be used for both Bochs and Qemu.

There is no vm state associated with the debugging port, simply
because it has none -- the entire interface is a single, stateless,
write-only port.

Most of the code was cribbed from the serial port driver.

Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 Makefile.target |    2 +-
 hw/debugcon.c   |  189 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pc.c         |    6 ++
 hw/pc.h         |    4 +
 qemu-common.h   |    1 +
 qemu-options.hx |   11 +++
 sysemu.h        |    6 ++
 vl.c            |   40 ++++++++++++
 8 files changed, 258 insertions(+), 1 deletions(-)
 create mode 100644 hw/debugcon.c

diff --git a/Makefile.target b/Makefile.target
index ab774e5..689cbf0 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -197,7 +197,7 @@ obj-i386-y += fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o parallel.o acpi.o piix_pci.o
 obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
-obj-i386-y += ne2000-isa.o
+obj-i386-y += ne2000-isa.o debugcon.o
 
 # shared objects
 obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
diff --git a/hw/debugcon.c b/hw/debugcon.c
new file mode 100644
index 0000000..6a724bb
--- /dev/null
+++ b/hw/debugcon.c
@@ -0,0 +1,189 @@
+/*
+ * QEMU Bochs-style debug console ("port E9") emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ * Copyright (c) Intel Corporation; author: H. Peter Anvin
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw.h"
+#include "qemu-char.h"
+#include "isa.h"
+#include "pc.h"
+
+//#define DEBUG_DEBUGCON
+
+struct DebugconState {
+    CharDriverState *chr;
+};
+
+typedef struct ISADebugconState {
+    ISADevice dev;
+    uint32_t index;
+    uint32_t iobase;
+    DebugconState state;
+} ISADebugconState;
+
+static void debugcon_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+{
+    DebugconState *s = opaque;
+    unsigned char ch = val;
+
+#ifdef DEBUG_DEBUGCON
+    printf("debugcon: write addr=0x%04x val=0x%02x\n", addr, val);
+#endif
+
+    qemu_chr_write(s->chr, &ch, 1);
+}
+
+static void debugcon_init_core(DebugconState *s)
+{
+    if (!s->chr) {
+        fprintf(stderr, "Can't create debugcon device, empty char device\n");
+	exit(1);
+    }
+
+    qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, s);
+}
+
+static const uint16_t isa_debugcon_iobase[MAX_DEBUGCON_PORTS] = { 0xe9 };
+
+static int debugcon_isa_initfn(ISADevice *dev)
+{
+    static int index;
+    ISADebugconState *isa = DO_UPCAST(ISADebugconState, dev, dev);
+    DebugconState *s = &isa->state;
+
+    if (isa->index == -1)
+        isa->index = index;
+    if (isa->index >= MAX_DEBUGCON_PORTS)
+        return -1;
+    if (isa->iobase == -1)
+	isa->iobase = isa_debugcon_iobase[isa->index];
+    index++;
+
+    debugcon_init_core(s);
+    register_ioport_write(isa->iobase, 1, 1, debugcon_ioport_write, s);
+    return 0;
+}
+
+DebugconState *debugcon_isa_init(int index, CharDriverState *chr)
+{
+    ISADevice *dev;
+
+    dev = isa_create("isa-debugcon");
+    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
+    if (qdev_init(&dev->qdev) < 0)
+        return NULL;
+    return &DO_UPCAST(ISADebugconState, dev, dev)->state;
+}
+
+#if 0 /* Non-ISA interfaces, available for the future */
+
+DebugconState *debugcon_init(int base, qemu_irq irq, int baudbase,
+                         CharDriverState *chr)
+{
+    DebugconState *s;
+
+    s = qemu_mallocz(sizeof(DebugconState));
+
+    s->chr = chr;
+    debugcon_init_core(s);
+
+    register_ioport_write(base, 8, 1, debugcon_ioport_write, s);
+    return s;
+}
+
+/* Memory mapped interface */
+static void debugcon_mm_writeb(void *opaque, target_phys_addr_t addr,
+                             uint32_t value)
+{
+    DebugconState *s = opaque;
+
+    debugcon_ioport_write(s, addr >> s->it_shift, value & 0xFF);
+}
+
+static void debugcon_mm_writew(void *opaque, target_phys_addr_t addr,
+                             uint32_t value)
+{
+    DebugconState *s = opaque;
+#ifdef TARGET_WORDS_BIGENDIAN
+    value = bswap16(value);
+#endif
+    debugcon_ioport_write(s, addr >> s->it_shift, value & 0xFFFF);
+}
+
+static void debugcon_mm_writel(void *opaque, target_phys_addr_t addr,
+                             uint32_t value)
+{
+    DebugconState *s = opaque;
+#ifdef TARGET_WORDS_BIGENDIAN
+    value = bswap32(value);
+#endif
+    debugcon_ioport_write(s, addr >> s->it_shift, value);
+}
+
+static CPUWriteMemoryFunc * const debugcon_mm_write[] = {
+    &debugcon_mm_writeb,
+    &debugcon_mm_writew,
+    &debugcon_mm_writel,
+};
+
+DebugconState *debugcon_mm_init (target_phys_addr_t base, int it_shift,
+                             qemu_irq irq, int baudbase,
+                             CharDriverState *chr, int ioregister)
+{
+    DebugconState *s;
+    int s_io_memory;
+
+    s = qemu_mallocz(sizeof(DebugconState));
+
+    s->chr = chr;
+
+    debugcon_init_core(s);
+
+    if (ioregister) {
+        s_io_memory = cpu_register_io_memory(NULL, debugcon_mm_write, s);
+        cpu_register_physical_memory(base, 1 << it_shift, s_io_memory);
+    }
+    return s;
+}
+
+#endif /* Non-ISA interfaces */
+
+static ISADeviceInfo debugcon_isa_info = {
+    .qdev.name  = "isa-debugcon",
+    .qdev.size  = sizeof(ISADebugconState),
+    .init       = debugcon_isa_initfn,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("index",  ISADebugconState, index,   -1),
+        DEFINE_PROP_HEX32("iobase", ISADebugconState, iobase,  -1),
+        DEFINE_PROP_CHR("chardev",  ISADebugconState, state.chr),
+        DEFINE_PROP_END_OF_LIST(),
+    },
+};
+
+static void debugcon_register_devices(void)
+{
+    isa_qdev_register(&debugcon_isa_info);
+}
+
+device_init(debugcon_register_devices)
diff --git a/hw/pc.c b/hw/pc.c
index 7c791c4..3957c78 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1159,6 +1159,12 @@ static void pc_init1(ram_addr_t ram_size,
         }
     }
 
+    for(i = 0; i < MAX_DEBUGCON_PORTS; i++) {
+        if (debugcon_hds[i]) {
+	    debugcon_isa_init(i, debugcon_hds[i]);
+        }
+    }
+
     for(i = 0; i < nb_nics; i++) {
         NICInfo *nd = &nd_table[i];
 
diff --git a/hw/pc.h b/hw/pc.h
index 03ffc91..4048be5 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -21,6 +21,10 @@ typedef struct ParallelState ParallelState;
 ParallelState *parallel_init(int index, CharDriverState *chr);
 ParallelState *parallel_mm_init(target_phys_addr_t base, int it_shift, qemu_irq irq, CharDriverState *chr);
 
+/* debugcon.c */
+
+DebugconState *debugcon_isa_init(int index, CharDriverState *chr);
+
 /* i8259.c */
 
 typedef struct PicState2 PicState2;
diff --git a/qemu-common.h b/qemu-common.h
index b779cfe..9dd3ca8 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -208,6 +208,7 @@ typedef struct uWireSlave uWireSlave;
 typedef struct I2SCodec I2SCodec;
 typedef struct DeviceState DeviceState;
 typedef struct SSIBus SSIBus;
+typedef struct DebugconState DebugconState;
 
 /* CPU save/load.  */
 void cpu_save(QEMUFile *f, void *opaque);
diff --git a/qemu-options.hx b/qemu-options.hx
index b65fd74..92ade30 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1586,6 +1586,17 @@ The default device is @code{vc} in graphical mode and @code{stdio} in
 non graphical mode.
 ETEXI
 
+DEF("debugcon", HAS_ARG, QEMU_OPTION_debugcon, \
+    "-debugcon dev   redirect the debug console to char device 'dev'\n")
+STEXI
+@item -monitor @var{dev}
+Redirect the debug console to host device @var{dev} (same devices as the
+serial port).  The debug console is an I/O port which is typically port
+0xe9; writing to that I/O port sends output to this device.
+The default device is @code{vc} in graphical mode and @code{stdio} in
+non graphical mode.
+ETEXI
+
 DEF("pidfile", HAS_ARG, QEMU_OPTION_pidfile, \
     "-pidfile file   write PID to 'file'\n")
 STEXI
diff --git a/sysemu.h b/sysemu.h
index b1887ef..e56b111 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -225,6 +225,12 @@ extern CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 
 extern CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
 
+/* debugging ports */
+
+#define MAX_DEBUGCON_PORTS 1
+
+extern CharDriverState *debugcon_hds[MAX_DEBUGCON_PORTS];
+
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
 
 #ifdef HAS_AUDIO
diff --git a/vl.c b/vl.c
index c3f3c8f..978f53b 100644
--- a/vl.c
+++ b/vl.c
@@ -212,6 +212,7 @@ int no_quit = 0;
 CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
+CharDriverState *debugcon_hds[MAX_DEBUGCON_PORTS];
 #ifdef TARGET_I386
 int win2k_install_hack = 0;
 int rtc_td_hack = 0;
@@ -4598,6 +4599,8 @@ int main(int argc, char **argv, char **envp)
     int parallel_device_index;
     const char *virtio_consoles[MAX_VIRTIO_CONSOLES];
     int virtio_console_index;
+    const char *debugcon_devices[MAX_DEBUGCON_PORTS];
+    int debugcon_device_index;
     const char *loadvm = NULL;
     QEMUMachine *machine;
     const char *cpu_model;
@@ -4683,6 +4686,11 @@ int main(int argc, char **argv, char **envp)
     }
     monitor_device_index = 0;
 
+    debugcon_devices[0] = "vc:80Cx24C";
+    for(i = 1; i < MAX_DEBUGCON_PORTS; i++)
+        debugcon_devices[i] = NULL;
+    debugcon_device_index = 0;
+
     for (i = 0; i < MAX_NODES; i++) {
         node_mem[i] = 0;
         node_cpumask[i] = 0;
@@ -5123,6 +5131,14 @@ int main(int argc, char **argv, char **envp)
                 serial_devices[serial_device_index] = optarg;
                 serial_device_index++;
                 break;
+	    case QEMU_OPTION_debugcon:
+                if (debugcon_device_index >= MAX_DEBUGCON_PORTS) {
+                    fprintf(stderr, "qemu: too many debugcon ports\n");
+                    exit(1);
+                }
+                debugcon_devices[debugcon_device_index] = optarg;
+                debugcon_device_index++;
+                break;
             case QEMU_OPTION_watchdog:
                 if (watchdog) {
                     fprintf(stderr,
@@ -5430,6 +5446,8 @@ int main(int argc, char **argv, char **envp)
        if (strncmp(monitor_devices[0], "vc", 2) == 0) {
            monitor_devices[0] = "stdio";
        }
+       if (debugcon_device_index == 0)
+           debugcon_devices[0] = "stdio";
     }
 
 #ifndef _WIN32
@@ -5691,6 +5709,20 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    for(i = 0; i < MAX_DEBUGCON_PORTS; i++) {
+        const char *devname = debugcon_devices[i];
+        if (devname && strcmp(devname, "none")) {
+            char label[32];
+            snprintf(label, sizeof(label), "debugcon%d", i);
+            debugcon_hds[i] = qemu_chr_open(label, devname, NULL);
+            if (!debugcon_hds[i]) {
+                fprintf(stderr, "qemu: could not open debugcon device '%s': %s\n",
+                        devname, strerror(errno));
+                exit(1);
+            }
+        }
+    }
+
     module_call_init(MODULE_INIT_DEVICE);
 
     if (watchdog) {
@@ -5826,6 +5858,14 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    for(i = 0; i < MAX_DEBUGCON_PORTS; i++) {
+        const char *devname = debugcon_devices[i];
+        if (devname && strcmp(devname, "none")) {
+            if (strstart(devname, "vc", 0))
+                qemu_chr_printf(debugcon_hds[i], "debugcon%d console\r\n", i);
+        }
+    }
+
     if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
         fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
                 gdbstub_dev);
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH] debugcon: support for debugging consoles (e.g.Bochs port 0xe9)

[Sebastian Herbszt]

H. Peter Anvin wrote:
> Add generic support for debugging consoles (simple I/O ports which
> when written to cause debugging output to be written to a target.)
> The current implementation matches Bochs' port 0xe9, allowing the same
> debugging code to be used for both Bochs and Qemu.

Your implementation seems to miss the read part, unless you don't want to be
compatible with Bochs. user.dbk says:
"Reading from port 0xE9 will will return 0xe9 to let you know if the feature is available."

- Sebastian

[Qemu-devel] Re: [PATCH] debugcon: support for debugging consoles (e.g.Bochs port 0xe9)

[H. Peter Anvin]

Sebastian Herbszt wrote:
> H. Peter Anvin wrote:
>> Add generic support for debugging consoles (simple I/O ports which
>> when written to cause debugging output to be written to a target.)
>> The current implementation matches Bochs' port 0xe9, allowing the same
>> debugging code to be used for both Bochs and Qemu.
> 
> Your implementation seems to miss the read part, unless you don't want 
> to be
> compatible with Bochs. user.dbk says:
> "Reading from port 0xE9 will will return 0xe9 to let you know if the 
> feature is available."
> 

I had actually missed that part... it seems somewhat pointless to me, 
but if that's the way Bochs does it it probably would indeed be better 
to be compatible.

	-hpa

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Gerd Hoffmann]

On 11/20/09 22:33, H. Peter Anvin wrote:

> +static const uint16_t isa_debugcon_iobase[MAX_DEBUGCON_PORTS] = { 0xe9 };

Not needed here.

> +    if (isa->index == -1)
> +        isa->index = index;
> +    if (isa->index>= MAX_DEBUGCON_PORTS)
> +        return -1;
> +    if (isa->iobase == -1)
> +	isa->iobase = isa_debugcon_iobase[isa->index];
> +    index++;

Also not needed.

> +    debugcon_init_core(s);
> +    register_ioport_write(isa->iobase, 1, 1, debugcon_ioport_write, s);
> +    return 0;
> +}
> +
> +DebugconState *debugcon_isa_init(int index, CharDriverState *chr)
> +{
> +    ISADevice *dev;
> +
> +    dev = isa_create("isa-debugcon");
> +    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
> +    if (qdev_init(&dev->qdev)<  0)
> +        return NULL;
> +    return&DO_UPCAST(ISADebugconState, dev, dev)->state;
> +}

Can be dropped too.

> +static ISADeviceInfo debugcon_isa_info = {
> +    .qdev.name  = "isa-debugcon",
> +    .qdev.size  = sizeof(ISADebugconState),
> +    .init       = debugcon_isa_initfn,
> +    .qdev.props = (Property[]) {
> +        DEFINE_PROP_HEX32("index",  ISADebugconState, index,   -1),

Drop index property.

> +        DEFINE_PROP_HEX32("iobase", ISADebugconState, iobase,  -1),

Last arg is the default value, simply place 0xe9 there.

> +        DEFINE_PROP_CHR("chardev",  ISADebugconState, state.chr),
> +        DEFINE_PROP_END_OF_LIST(),
> +    },
> +};
> +
> +static void debugcon_register_devices(void)
> +{
> +    isa_qdev_register(&debugcon_isa_info);
> +}

> diff --git a/hw/pc.c b/hw/pc.c
> diff --git a/hw/pc.h b/hw/pc.h
> diff --git a/qemu-common.h b/qemu-common.h
> diff --git a/qemu-options.hx b/qemu-options.hx
> diff --git a/sysemu.h b/sysemu.h
> diff --git a/vl.c b/vl.c

All not needed.  Simply registering as qdev device is enougth.  You can 
then add a debug port like this, without adding new cmd line options:

   -chardev vc,id=debug -device isa-debugcon,chardev=debug

Adding a second one on a non-default port works this way:

   -chardev vc,id=d2 -device isa-debugcon,iobase=0xea,chardev=d2

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[H. Peter Anvin]

On 11/23/2009 02:52 AM, Gerd Hoffmann wrote:
> 
> All not needed.  Simply registering as qdev device is enougth.  You can 
> then add a debug port like this, without adding new cmd line options:
> 
>    -chardev vc,id=debug -device isa-debugcon,chardev=debug
> 
> Adding a second one on a non-default port works this way:
> 
>    -chardev vc,id=d2 -device isa-debugcon,iobase=0xea,chardev=d2
> 

Sure, but is that user friendly?

	-hpa

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[H. Peter Anvin]

On 11/23/2009 12:11 PM, H. Peter Anvin wrote:
> On 11/23/2009 02:52 AM, Gerd Hoffmann wrote:
>>
>> All not needed.  Simply registering as qdev device is enougth.  You can 
>> then add a debug port like this, without adding new cmd line options:
>>
>>    -chardev vc,id=debug -device isa-debugcon,chardev=debug
>>
>> Adding a second one on a non-default port works this way:
>>
>>    -chardev vc,id=d2 -device isa-debugcon,iobase=0xea,chardev=d2
>>
> 
> Sure, but is that user friendly?
> 

Question still stands... if this is The Way It Is Done, then I'll do it
that way, but I will personally never be able to remember to type the
above stuff without looking it up every time, whereas "-debugcon stdio"
is something I can remember in my sleep.

	-hpa

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Alexander Graf]

On 25.11.2009, at 00:05, H. Peter Anvin wrote:

> On 11/23/2009 12:11 PM, H. Peter Anvin wrote:
>> On 11/23/2009 02:52 AM, Gerd Hoffmann wrote:
>>> 
>>> All not needed.  Simply registering as qdev device is enougth.  You can 
>>> then add a debug port like this, without adding new cmd line options:
>>> 
>>>   -chardev vc,id=debug -device isa-debugcon,chardev=debug
>>> 
>>> Adding a second one on a non-default port works this way:
>>> 
>>>   -chardev vc,id=d2 -device isa-debugcon,iobase=0xea,chardev=d2
>>> 
>> 
>> Sure, but is that user friendly?
>> 
> 
> Question still stands... if this is The Way It Is Done, then I'll do it
> that way, but I will personally never be able to remember to type the
> above stuff without looking it up every time, whereas "-debugcon stdio"
> is something I can remember in my sleep.

I agree with hpa here. While the -device syntax is nice for development and hacky stuff, let's please not force any user to use it!

Alex

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Gerd Hoffmann]

On 11/25/09 00:05, H. Peter Anvin wrote:
> On 11/23/2009 12:11 PM, H. Peter Anvin wrote:
>> On 11/23/2009 02:52 AM, Gerd Hoffmann wrote:
>>>
>>> All not needed.  Simply registering as qdev device is enougth.  You can
>>> then add a debug port like this, without adding new cmd line options:
>>>
>>>     -chardev vc,id=debug -device isa-debugcon,chardev=debug
>>>
>>> Adding a second one on a non-default port works this way:
>>>
>>>     -chardev vc,id=d2 -device isa-debugcon,iobase=0xea,chardev=d2
>>>
>>
>> Sure, but is that user friendly?
>>
>
> Question still stands... if this is The Way It Is Done, then I'll do it
> that way, but I will personally never be able to remember to type the
> above stuff without looking it up every time, whereas "-debugcon stdio"
> is something I can remember in my sleep.

You can stick it into a config file[1] like this:

[chardev "debuglog"]
   backend = "file"
   path = "/path/to/debug.log"

[device]
   driver = "isa-debugcon"
   chardev = "debuglog"

Then use "qemu -readconfig $file".

cheers,
   Gerd

[1] Well, almost, fix to make this actually work went to the list today.

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[H. Peter Anvin]

Gerd Hoffmann wrote:
> 
> You can stick it into a config file[1] like this:
> 
> [chardev "debuglog"]
>   backend = "file"
>   path = "/path/to/debug.log"
> 
> [device]
>   driver = "isa-debugcon"
>   chardev = "debuglog"
> 
> Then use "qemu -readconfig $file".
> 

Great... at least in my opinion, one of the things that makes Qemu so 
much easier to use than Bochs is that one has a comprehensive (and 
comprehensable) command line and don't have to set up a configuration 
file for every trivial little thing.  It's a good thing for long-living 
virtualization instances, but I would be sad if the command line will 
end up being neglected.

	-hpa

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Gerd Hoffmann]

On 11/25/09 19:01, H. Peter Anvin wrote:
> Gerd Hoffmann wrote:
>>
>> You can stick it into a config file[1] like this:
>>
>> [chardev "debuglog"]
>> backend = "file"
>> path = "/path/to/debug.log"
>>
>> [device]
>> driver = "isa-debugcon"
>> chardev = "debuglog"
>>
>> Then use "qemu -readconfig $file".
>>
>
> Great... at least in my opinion, one of the things that makes Qemu so
> much easier to use than Bochs is that one has a comprehensive (and
> comprehensable) command line and don't have to set up a configuration
> file for every trivial little thing. It's a good thing for long-living
> virtualization instances, but I would be sad if the command line will
> end up being neglected.

You can happily mixup things as you like.  Use pure command line.  Use 
pure config file (well, at least is that the plan, it doesn't fully work 
yet).  Mix cmd line switches and config file.  Reading multiple config 
files works just fine too, so you can split your vm config into multiple 
parts and enable/disable parts by reading/not reading the file in 
question ...

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[H. Peter Anvin]

On 11/25/2009 02:21 PM, Gerd Hoffmann wrote:
> 
> You can happily mixup things as you like.  Use pure command line.  Use 
> pure config file (well, at least is that the plan, it doesn't fully work 
> yet).  Mix cmd line switches and config file.  Reading multiple config 
> files works just fine too, so you can split your vm config into multiple 
> parts and enable/disable parts by reading/not reading the file in 
> question ...
> 

Right, of course; I didn't mean to imply that your config file would
make the command line be unavailable, nor did I mean to imply that it
isn't useful (quite on the contrary.)

What I meant was that the presence of config file does not avoid the
user-hostility of options like "-chardev stdio,id=debug -device
isa-debugcon,chardev=debug" ... for a command line user, the situation
is still the same, and when using Qemu for debugging and testing, this
is one case when you would be likely to run into the command line.

As such, I think there is still a place for the simple command line
options like "-hda", "-serial" and my proposed "-debugcon", in addition
to the full-featured connect-anything options and configuration files.
That's all.

	-hpa

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[H. Peter Anvin]

On 11/25/2009 02:26 PM, H. Peter Anvin wrote:
> 
> As such, I think there is still a place for the simple command line
> options like "-hda", "-serial" and my proposed "-debugcon", in addition
> to the full-featured connect-anything options and configuration files.
> That's all.
> 

I guess one question is if the simplified options could be better
implemented via some kind of builtin macros or translation submodule
rather than being treated as first-class options which they basically
are now.  Still, that's a future development task.

	-hpa

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Gerd Hoffmann]

On 11/25/09 23:31, H. Peter Anvin wrote:
> On 11/25/2009 02:26 PM, H. Peter Anvin wrote:
>>
>> As such, I think there is still a place for the simple command line
>> options like "-hda", "-serial" and my proposed "-debugcon", in addition
>> to the full-featured connect-anything options and configuration files.
>> That's all.
>>
>
> I guess one question is if the simplified options could be better
> implemented via some kind of builtin macros or translation submodule
> rather than being treated as first-class options which they basically
> are now.  Still, that's a future development task.

Wouldn't be that hard I think.  Something along the lines of

     case QEMU_OPTION_debugcon:
          opts = qemu_chr_parse_compat("debugcon", optarg);
          if (NULL == opts) {
               fprintf(stderr, "already have a debugcon chardev\");
               exit(1);
          }
          opts = qemu_opts_create(&qemu_device_opts, "debugcon", 1);
          if (NULL == opts) {
               fprintf(stderr, "already have a debugcon device\");
               exit(1);
          }
          qemu_opt_set(opts, "driver", isa-debugcon);
          qemu_opt_set(opts, "chardev", "debugcon");
          break;

should do the trick.  Depends on the chardev fix mentioned in this 
thread too.  Allows to define a single device with the default port via 
shortcut.

I tend to not add new command line switches for every little new device. 
  For this one it might be reasonable nevertheless given the use cases.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On 11/25/09 23:31, H. Peter Anvin wrote:
>> On 11/25/2009 02:26 PM, H. Peter Anvin wrote:
>>>
>>> As such, I think there is still a place for the simple command line
>>> options like "-hda", "-serial" and my proposed "-debugcon", in addition
>>> to the full-featured connect-anything options and configuration files.
>>> That's all.
>>>
>>
>> I guess one question is if the simplified options could be better
>> implemented via some kind of builtin macros or translation submodule
>> rather than being treated as first-class options which they basically
>> are now.  Still, that's a future development task.
>
> Wouldn't be that hard I think.  Something along the lines of
>
>     case QEMU_OPTION_debugcon:
>          opts = qemu_chr_parse_compat("debugcon", optarg);
>          if (NULL == opts) {
>               fprintf(stderr, "already have a debugcon chardev\");
>               exit(1);
>          }
>          opts = qemu_opts_create(&qemu_device_opts, "debugcon", 1);
>          if (NULL == opts) {
>               fprintf(stderr, "already have a debugcon device\");
>               exit(1);
>          }
>          qemu_opt_set(opts, "driver", isa-debugcon);
>          qemu_opt_set(opts, "chardev", "debugcon");
>          break;
>
> should do the trick.  Depends on the chardev fix mentioned in this
> thread too.  Allows to define a single device with the default port
> via shortcut.

There's precedence for this method to implement convenience options;
look for uses of qemu_opts_create() / qemu_opt_set().  Expanding the
convenience option into longhand that way saves code and avoids subtle
differences between the two.  Just avoid NULL == opts kraxelism ;)

[...]

[Qemu-devel] Re: [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Paolo Bonzini]

On 11/25/2009 11:54 PM, Gerd Hoffmann wrote:
>
> Wouldn't be that hard I think.

Cool.  I had thought of a similar exercise to wet my feet with 
qdev/qemuopts, I may actually do it when I have some time...

Paolo

[Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[H. Peter Anvin]

From: H. Peter Anvin <hpa@zytor.com>

Add generic support for debugging consoles (simple I/O ports which
when written to cause debugging output to be written to a target.)
The current implementation matches Bochs' port 0xe9, allowing the same
debugging code to be used for both Bochs and Qemu.

There is no vm state associated with the debugging port, simply
because it has none -- the entire interface is a single, stateless,
write-only port.

Most of the code was cribbed from the serial port driver.

v2: removed non-ISA variants (they can be introduced when/if someone
wants them, using code from the serial port); added configurable
readback (Bochs returns 0xe9 on a read from this register, mimic that
by default)  This retains the apparently somewhat controversial user
friendly option, however.

Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 Makefile.target |    2 +-
 hw/debugcon.c   |  118 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pc.c         |    6 +++
 hw/pc.h         |    4 ++
 qemu-common.h   |    1 +
 qemu-options.hx |   11 +++++
 sysemu.h        |    6 +++
 vl.c            |   40 +++++++++++++++++++
 8 files changed, 187 insertions(+), 1 deletions(-)
 create mode 100644 hw/debugcon.c

diff --git a/Makefile.target b/Makefile.target
index 2985658..df4a7b2 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -197,7 +197,7 @@ obj-i386-y += fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o parallel.o acpi.o piix_pci.o
 obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
-obj-i386-y += ne2000-isa.o
+obj-i386-y += ne2000-isa.o debugcon.o
 
 # shared objects
 obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
diff --git a/hw/debugcon.c b/hw/debugcon.c
new file mode 100644
index 0000000..a0c2923
--- /dev/null
+++ b/hw/debugcon.c
@@ -0,0 +1,118 @@
+/*
+ * QEMU Bochs-style debug console ("port E9") emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ * Copyright (c) Intel Corporation; author: H. Peter Anvin
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw.h"
+#include "qemu-char.h"
+#include "isa.h"
+#include "pc.h"
+
+//#define DEBUG_DEBUGCON
+
+struct DebugconState {
+    CharDriverState *chr;
+    uint32_t readback;
+};
+
+typedef struct ISADebugconState {
+    ISADevice dev;
+    uint32_t iobase;
+    DebugconState state;
+} ISADebugconState;
+
+static void debugcon_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+{
+    DebugconState *s = opaque;
+    unsigned char ch = val;
+
+#ifdef DEBUG_DEBUGCON
+    printf("debugcon: write addr=0x%04x val=0x%02x\n", addr, val);
+#endif
+
+    qemu_chr_write(s->chr, &ch, 1);
+}
+
+
+static uint32_t debugcon_ioport_read(void *opaque, uint32_t addr)
+{
+    DebugconState *s = opaque;
+
+#ifdef DEBUG_DEBUGCON
+    printf("debugcon: read addr=0x%04x\n", addr, val);
+#endif
+
+    return s->readback;
+}
+    
+static void debugcon_init_core(DebugconState *s)
+{
+    if (!s->chr) {
+        fprintf(stderr, "Can't create debugcon device, empty char device\n");
+	exit(1);
+    }
+
+    qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, s);
+}
+
+static int debugcon_isa_initfn(ISADevice *dev)
+{
+    ISADebugconState *isa = DO_UPCAST(ISADebugconState, dev, dev);
+    DebugconState *s = &isa->state;
+
+    debugcon_init_core(s);
+    register_ioport_write(isa->iobase, 1, 1, debugcon_ioport_write, s);
+    register_ioport_read(isa->iobase, 1, 1, debugcon_ioport_read, s);
+    return 0;
+}
+
+DebugconState *debugcon_isa_init(int index, CharDriverState *chr)
+{
+    ISADevice *dev;
+
+    dev = isa_create("isa-debugcon");
+    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
+    if (qdev_init(&dev->qdev) < 0)
+        return NULL;
+    return &DO_UPCAST(ISADebugconState, dev, dev)->state;
+}
+
+static ISADeviceInfo debugcon_isa_info = {
+    .qdev.name  = "isa-debugcon",
+    .qdev.size  = sizeof(ISADebugconState),
+    .init       = debugcon_isa_initfn,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", ISADebugconState, iobase, 0xe9),
+        DEFINE_PROP_CHR("chardev",  ISADebugconState, state.chr),
+	DEFINE_PROP_HEX32("readback", ISADebugconState, state.readback, 0xe9),
+        DEFINE_PROP_END_OF_LIST(),
+    },
+};
+
+static void debugcon_register_devices(void)
+{
+    isa_qdev_register(&debugcon_isa_info);
+}
+
+device_init(debugcon_register_devices)
diff --git a/hw/pc.c b/hw/pc.c
index 7c791c4..3957c78 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1159,6 +1159,12 @@ static void pc_init1(ram_addr_t ram_size,
         }
     }
 
+    for(i = 0; i < MAX_DEBUGCON_PORTS; i++) {
+        if (debugcon_hds[i]) {
+	    debugcon_isa_init(i, debugcon_hds[i]);
+        }
+    }
+
     for(i = 0; i < nb_nics; i++) {
         NICInfo *nd = &nd_table[i];
 
diff --git a/hw/pc.h b/hw/pc.h
index 03ffc91..4048be5 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -21,6 +21,10 @@ typedef struct ParallelState ParallelState;
 ParallelState *parallel_init(int index, CharDriverState *chr);
 ParallelState *parallel_mm_init(target_phys_addr_t base, int it_shift, qemu_irq irq, CharDriverState *chr);
 
+/* debugcon.c */
+
+DebugconState *debugcon_isa_init(int index, CharDriverState *chr);
+
 /* i8259.c */
 
 typedef struct PicState2 PicState2;
diff --git a/qemu-common.h b/qemu-common.h
index b1e038b..4c6e745 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -208,6 +208,7 @@ typedef struct uWireSlave uWireSlave;
 typedef struct I2SCodec I2SCodec;
 typedef struct DeviceState DeviceState;
 typedef struct SSIBus SSIBus;
+typedef struct DebugconState DebugconState;
 
 /* CPU save/load.  */
 void cpu_save(QEMUFile *f, void *opaque);
diff --git a/qemu-options.hx b/qemu-options.hx
index b65fd74..92ade30 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1586,6 +1586,17 @@ The default device is @code{vc} in graphical mode and @code{stdio} in
 non graphical mode.
 ETEXI
 
+DEF("debugcon", HAS_ARG, QEMU_OPTION_debugcon, \
+    "-debugcon dev   redirect the debug console to char device 'dev'\n")
+STEXI
+@item -monitor @var{dev}
+Redirect the debug console to host device @var{dev} (same devices as the
+serial port).  The debug console is an I/O port which is typically port
+0xe9; writing to that I/O port sends output to this device.
+The default device is @code{vc} in graphical mode and @code{stdio} in
+non graphical mode.
+ETEXI
+
 DEF("pidfile", HAS_ARG, QEMU_OPTION_pidfile, \
     "-pidfile file   write PID to 'file'\n")
 STEXI
diff --git a/sysemu.h b/sysemu.h
index b1887ef..e56b111 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -225,6 +225,12 @@ extern CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 
 extern CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
 
+/* debugging ports */
+
+#define MAX_DEBUGCON_PORTS 1
+
+extern CharDriverState *debugcon_hds[MAX_DEBUGCON_PORTS];
+
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
 
 #ifdef HAS_AUDIO
diff --git a/vl.c b/vl.c
index ee43808..ce620c2 100644
--- a/vl.c
+++ b/vl.c
@@ -212,6 +212,7 @@ int no_quit = 0;
 CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
+CharDriverState *debugcon_hds[MAX_DEBUGCON_PORTS];
 #ifdef TARGET_I386
 int win2k_install_hack = 0;
 int rtc_td_hack = 0;
@@ -4598,6 +4599,8 @@ int main(int argc, char **argv, char **envp)
     int parallel_device_index;
     const char *virtio_consoles[MAX_VIRTIO_CONSOLES];
     int virtio_console_index;
+    const char *debugcon_devices[MAX_DEBUGCON_PORTS];
+    int debugcon_device_index;
     const char *loadvm = NULL;
     QEMUMachine *machine;
     const char *cpu_model;
@@ -4683,6 +4686,11 @@ int main(int argc, char **argv, char **envp)
     }
     monitor_device_index = 0;
 
+    debugcon_devices[0] = "vc:80Cx24C";
+    for(i = 1; i < MAX_DEBUGCON_PORTS; i++)
+        debugcon_devices[i] = NULL;
+    debugcon_device_index = 0;
+
     for (i = 0; i < MAX_NODES; i++) {
         node_mem[i] = 0;
         node_cpumask[i] = 0;
@@ -5121,6 +5129,14 @@ int main(int argc, char **argv, char **envp)
                 serial_devices[serial_device_index] = optarg;
                 serial_device_index++;
                 break;
+	    case QEMU_OPTION_debugcon:
+                if (debugcon_device_index >= MAX_DEBUGCON_PORTS) {
+                    fprintf(stderr, "qemu: too many debugcon ports\n");
+                    exit(1);
+                }
+                debugcon_devices[debugcon_device_index] = optarg;
+                debugcon_device_index++;
+                break;
             case QEMU_OPTION_watchdog:
                 if (watchdog) {
                     fprintf(stderr,
@@ -5428,6 +5444,8 @@ int main(int argc, char **argv, char **envp)
        if (strncmp(monitor_devices[0], "vc", 2) == 0) {
            monitor_devices[0] = "stdio";
        }
+       if (debugcon_device_index == 0)
+           debugcon_devices[0] = "stdio";
     }
 
 #ifndef _WIN32
@@ -5689,6 +5707,20 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    for(i = 0; i < MAX_DEBUGCON_PORTS; i++) {
+        const char *devname = debugcon_devices[i];
+        if (devname && strcmp(devname, "none")) {
+            char label[32];
+            snprintf(label, sizeof(label), "debugcon%d", i);
+            debugcon_hds[i] = qemu_chr_open(label, devname, NULL);
+            if (!debugcon_hds[i]) {
+                fprintf(stderr, "qemu: could not open debugcon device '%s': %s\n",
+                        devname, strerror(errno));
+                exit(1);
+            }
+        }
+    }
+
     module_call_init(MODULE_INIT_DEVICE);
 
     if (watchdog) {
@@ -5824,6 +5856,14 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    for(i = 0; i < MAX_DEBUGCON_PORTS; i++) {
+        const char *devname = debugcon_devices[i];
+        if (devname && strcmp(devname, "none")) {
+            if (strstart(devname, "vc", 0))
+                qemu_chr_printf(debugcon_hds[i], "debugcon%d console\r\n", i);
+        }
+    }
+
     if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
         fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
                 gdbstub_dev);
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 522 bytes --]

H. Peter Anvin wrote:
...
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b65fd74..92ade30 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1586,6 +1586,17 @@ The default device is @code{vc} in graphical mode and @code{stdio} in
>  non graphical mode.
>  ETEXI
>  
> +DEF("debugcon", HAS_ARG, QEMU_OPTION_debugcon, \
> +    "-debugcon dev   redirect the debug console to char device 'dev'\n")
> +STEXI
> +@item -monitor @var{dev}
          ^^^^^^^
Classic copy&paste mistake.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Gerd Hoffmann]

On 12/29/09 22:51, H. Peter Anvin wrote:
> Add generic support for debugging consoles (simple I/O ports which
> when written to cause debugging output to be written to a target.)
> The current implementation matches Bochs' port 0xe9, allowing the same
> debugging code to be used for both Bochs and Qemu.

Looks good to me.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
   Gerd