Re: [Qemu-devel] [PATCH] Implement PC port80 debug register.

[Anthony Liguori <anthony@codemonkey.ws>]

Jordan Justen wrote:
> In PC systems, the byte I/O port 0x80 is commonly written to
> by BIOS and/or system software as a simple checkpoint method.
>
> This change adds an 'info port80' monitor command to retrieve
> the last value written out to port80.
>   

Avi had suggested something like info debugreg.  I think something like 
that would be better as using the name "port80" makes it a very i386 
centric monitor option.

>  
>      /* init basic PC hardware */
> -    register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
> +    port80_init();
>   

We really ought to make this a SysBus qdev device.  To do this, right 
here you would call:

sysbus_create_simple("pc,port80", -1, NULL);

>      register_ioport_write(0xf0, 1, 1, ioportF0_write, NULL);
>  
> diff --git a/hw/pc.h b/hw/pc.h
> index 9fbae20..1d0423e 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -166,4 +166,11 @@ void pci_piix4_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn,
>  void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd);
>  
>  int cpu_is_bsp(CPUState *env);
> +
> +/* port80.c */
> +
> +typedef struct Port80State Port80State;
> +
> +Port80State *port80_init(void);
> +
>   

You would no longer need this.

>  #endif
> diff --git a/hw/port80.c b/hw/port80.c
> new file mode 100644
> index 0000000..947b3cd
> --- /dev/null
> +++ b/hw/port80.c
> @@ -0,0 +1,104 @@
> +/*
> + * QEMU debug port 80 emulation
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + * Copyright (c) 2009 Jordan Justen
> + *
> + * 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 "sysemu.h"
> +#include "pc.h"
> +#include "isa.h"
> +#include "monitor.h"
> +
> +void do_monitor_info_port80(Monitor *mon);
> +
> +//#define DEBUG_PORT80
> +//#define PORT80_READ_SUPPORT
>   
I'd rather not have the device model change based on #defines.  Either 
remove read support or enable it unconditionally.

> +struct Port80State {
> +    uint8_t data;
> +};

Add a SysBusDevice to this.


> +Port80State *port80_init()
>   

This becomes:

static void port80_init(SysBusDevice *dev)

> +{
> +    Port80State *s;
> +
> +    s = qemu_mallocz(sizeof(Port80State));
> +    state = s;
>   

This becomes:

s = FROM_SYSBUS(Port80State, dev);

> +    register_ioport_write(0x80, 1, 1, port80_ioport_write, s);
> +#ifdef PORT80_READ_SUPPORT
> +    register_ioport_read(0x80, 1, 1, port80_ioport_read, s);
> +#endif
> +
> +    register_savevm("port80", 0x80, 1, port80_save, port80_load, s);
> +    return s;
> +}
>   

Then you need to add a device_init() that calls sysbus_register_dev().

Regards,

Anthony Liguori