Re: [PATCH 05/21] q800: add IOSB subsystem

[Philippe Mathieu-Daudé]

On 2/7/23 17:48, Mark Cave-Ayland wrote:
> It is needed because it defines the BIOSConfig area.
>
> Co-developed-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   MAINTAINERS            |   2 +
>   hw/m68k/Kconfig        |   1 +
>   hw/m68k/q800.c         |   9 +++
>   hw/misc/Kconfig        |   3 +
>   hw/misc/iosb.c         | 137 +++++++++++++++++++++++++++++++++++++++++
>   hw/misc/meson.build    |   1 +
>   hw/misc/trace-events   |   4 ++
>   include/hw/m68k/q800.h |   2 +
>   include/hw/misc/iosb.h |  25 ++++++++
>   9 files changed, 184 insertions(+)
>   create mode 100644 hw/misc/iosb.c
>   create mode 100644 include/hw/misc/iosb.h


> diff --git a/hw/misc/iosb.c b/hw/misc/iosb.c
> new file mode 100644
> index 0000000000..4fc10bcf9f
> --- /dev/null
> +++ b/hw/misc/iosb.c
> @@ -0,0 +1,137 @@
> +/*
> + * QEMU IOSB emulation
> + *
> + * Copyright (c) 2019 Laurent Vivier
> + * Copyright (c) 2022 Mark Cave-Ayland
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "migration/vmstate.h"
> +#include "hw/sysbus.h"
> +#include "hw/misc/iosb.h"
> +#include "trace.h"
> +
> +#define IOSB_SIZE          0x2000
> +
> +#define IOSB_CONFIG        0x0
> +#define IOSB_CONFIG2       0x100
> +#define IOSB_SONIC_SCSI    0x200
> +#define IOSB_REVISION      0x300
> +#define IOSB_SCSI_RESID    0x400
> +#define IOSB_BRIGHTNESS    0x500
> +#define IOSB_TIMEOUT       0x600
> +
> +
> +static uint64_t iosb_read(void *opaque, hwaddr addr,
> +                          unsigned size)
> +{
> +    IOSBState *s = IOSB(opaque);
> +    uint64_t val = 0;
> +
> +    switch (addr) {
> +    case IOSB_CONFIG:
> +    case IOSB_CONFIG2:
> +    case IOSB_SONIC_SCSI:
> +    case IOSB_REVISION:
> +    case IOSB_SCSI_RESID:
> +    case IOSB_BRIGHTNESS:
> +    case IOSB_TIMEOUT:
> +        val = s->regs[addr >> 8];
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "IOSB: unimplemented read addr=0x%"PRIx64
> +                                 " val=0x%"PRIx64 " size=%d\n",
> +                                 addr, val, size);
> +    }
> +
> +    trace_iosb_read(addr, val, size);
> +    return val;
> +}
> +
> +static void iosb_write(void *opaque, hwaddr addr, uint64_t val,
> +                       unsigned size)
> +{
> +    IOSBState *s = IOSB(opaque);
> +
> +    switch (addr) {
> +    case IOSB_CONFIG:
> +    case IOSB_CONFIG2:
> +    case IOSB_SONIC_SCSI:
> +    case IOSB_REVISION:
> +    case IOSB_SCSI_RESID:
> +    case IOSB_BRIGHTNESS:
> +    case IOSB_TIMEOUT:
> +        s->regs[addr >> 8] = val;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "IOSB: unimplemented write addr=0x%"PRIx64
> +                                 " val=0x%"PRIx64 " size=%d\n",
> +                                 addr, val, size);
> +    }
> +
> +    trace_iosb_write(addr, val, size);
> +}
> +
> +static const MemoryRegionOps iosb_mmio_ops = {
> +    .read = iosb_read,
> +    .write = iosb_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .impl = {
> +        .min_access_size = 1,

IIUC .impl.min_access_size = 4.

Do you mean .valid.min_access_size = 1?

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        .max_access_size = 4,
> +    },
> +};