[Qemu-devel] Re: [PATCH] pc: move port 92 stuff back to pc.c from pckbd.c

[Peter Lieven]

Am 28.12.2010 um 22:24 schrieb Blue Swirl:

> 956a3e6bb7386de48b642d4fee11f7f86a2fcf9a introduced a bug concerning
> reset bit for port 92.
> 
> Since the keyboard output port and port 92 are not compatible anyway,
> let's separate them.
> 
> Reported-by: Peter Lieven <address@hidden>
> Signed-off-by: Blue Swirl <address@hidden>
> ---
> hw/pc.c    |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> hw/pckbd.c |   19 +---------------
> 2 files changed, 67 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 18a4a9f..e63b397 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -411,11 +411,71 @@ void pc_cmos_init(ram_addr_t ram_size,
> ram_addr_t above_4g_mem_size,
>     qemu_register_reset(pc_cmos_init_late, &arg);
> }
> 
> +/* port 92 stuff: could be split off */
> +typedef struct Port92State {
> +    ISADevice dev;
> +    uint8_t outport;
> +    qemu_irq *a20_out;
> +} Port92State;
> +
> +static void port92_write(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    Port92State *s = opaque;
> +
> +    DPRINTF("port92: write 0x%02x\n", val);
> +    s->outport = val;
> +    qemu_set_irq(*s->a20_out, (val >> 1) & 1);
> +    if (val & 1) {
> +        qemu_system_reset_request();
> +    }
> +}
> +
> +static uint32_t port92_read(void *opaque, uint32_t addr)
> +{
> +    Port92State *s = opaque;
> +    uint32_t ret;
> +
> +    ret = s->outport;
> +    DPRINTF("port92: read 0x%02x\n", ret);
> +    return ret;
> +}
> +
> +static void port92_init(ISADevice *dev, qemu_irq *a20_out)
> +{
> +    Port92State *s = DO_UPCAST(Port92State, dev, dev);
> +
> +    s->a20_out = a20_out;
> +}
> +
> +static int port92_initfn(ISADevice *dev)
> +{
> +    Port92State *s = DO_UPCAST(Port92State, dev, dev);
> +
> +    register_ioport_read(0x92, 1, 1, port92_read, s);
> +    register_ioport_write(0x92, 1, 1, port92_write, s);
> +    isa_init_ioport(dev, 0x92);
> +    return 0;
> +}
> +
> +static ISADeviceInfo port92_info = {
> +    .qdev.name     = "port92",
> +    .qdev.size     = sizeof(Port92State),
> +    .qdev.no_user  = 1,
> +    .init          = port92_initfn,
> +};
> +
> +static void port92_register(void)
> +{
> +    isa_qdev_register(&port92_info);
> +}
> +device_init(port92_register)
> +
> static void handle_a20_line_change(void *opaque, int irq, int level)
> {
>     CPUState *cpu = opaque;
> 
>     /* XXX: send to all CPUs ? */
> +    /* XXX: add logic to handle multiple A20 line sources */
>     cpu_x86_set_a20(cpu, level);
> }
> 
> @@ -1027,7 +1087,7 @@ void pc_basic_device_init(qemu_irq *isa_irq,
>     PITState *pit;
>     qemu_irq rtc_irq = NULL;
>     qemu_irq *a20_line;
> -    ISADevice *i8042;
> +    ISADevice *i8042, *port92;
>     qemu_irq *cpu_exit_irq;
> 
>     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
> @@ -1061,10 +1121,12 @@ void pc_basic_device_init(qemu_irq *isa_irq,
>         }
>     }
> 
> -    a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 1);
> +    a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
>     i8042 = isa_create_simple("i8042");
> -    i8042_setup_a20_line(i8042, a20_line);
> +    i8042_setup_a20_line(i8042, &a20_line[0]);
>     vmmouse_init(i8042);
> +    port92 = isa_create_simple("port92");
> +    port92_init(port92, &a20_line[1]);
> 
>     cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
>     DMA_init(0, cpu_exit_irq);
> diff --git a/hw/pckbd.c b/hw/pckbd.c
> index 863b485..958de0a 100644
> --- a/hw/pckbd.c
> +++ b/hw/pckbd.c
> @@ -211,10 +211,8 @@ static void kbd_queue(KBDState *s, int b, int aux)
>         ps2_queue(s->kbd, b);
> }
> 
> -static void ioport92_write(void *opaque, uint32_t addr, uint32_t val)
> +static void outport_write(KBDState *s, uint32_t addr, uint32_t val)
> {
> -    KBDState *s = opaque;
> -
>     DPRINTF("kbd: write outport=0x%02x\n", val);
>     s->outport = val;
>     if (s->a20_out) {
> @@ -225,16 +223,6 @@ static void ioport92_write(void *opaque, uint32_t
> addr, uint32_t val)
>     }
> }
> 
> -static uint32_t ioport92_read(void *opaque, uint32_t addr)
> -{
> -    KBDState *s = opaque;
> -    uint32_t ret;
> -
> -    ret = s->outport;
> -    DPRINTF("kbd: read outport=0x%02x\n", ret);
> -    return ret;
> -}
> -
> static void kbd_write_command(void *opaque, uint32_t addr, uint32_t val)
> {
>     KBDState *s = opaque;
> @@ -357,7 +345,7 @@ static void kbd_write_data(void *opaque, uint32_t
> addr, uint32_t val)
>         kbd_queue(s, val, 1);
>         break;
>     case KBD_CCMD_WRITE_OUTPORT:
> -        ioport92_write(s, 0, val);
> +        outport_write(s, 0, val);
>         break;
>     case KBD_CCMD_WRITE_MOUSE:
>         ps2_write_mouse(s->mouse, val);
> @@ -489,9 +477,6 @@ static int i8042_initfn(ISADevice *dev)
>     register_ioport_read(0x64, 1, 1, kbd_read_status, s);
>     register_ioport_write(0x64, 1, 1, kbd_write_command, s);
>     isa_init_ioport(dev, 0x64);
> -    register_ioport_read(0x92, 1, 1, ioport92_read, s);
> -    register_ioport_write(0x92, 1, 1, ioport92_write, s);
> -    isa_init_ioport(dev, 0x92);
> 
>     s->kbd = ps2_kbd_init(kbd_update_kbd_irq, s);
>     s->mouse = ps2_mouse_init(kbd_update_aux_irq, s);
> -- 
> 1.6.2.4

What happens if I set the A20 Gate via the keyboard controller and then read 
its status via ioport 0x92? This doesn't work, or does it?
What was wrong reading the A20 via

int ioport_get_a20(void)
{
    return ((first_cpu->a20_mask >> 20) & 1);
}

every time ioport 0x92 or the keyobard outport is read?

I also found somewhere that bit 0 (reset) of port 0x92 and the keyboard 
controller is read only.
Maybe it should be set by default to 0 in the port92_read and to 1 when the 
keyboard outport is read.

Peter