Re: [PATCH v7 0/8] Pegasos2 emulation

[BALATON Zoltan <balaton@eik.bme.hu>]

Another arrempt to explain patch 1. This is the via-superio class that's a 
subclass of ISA superio:

https://github.com/patchew-project/qemu/blob/ca5d88d2fee0016f939e91ae8b32c18e682064fa/hw/isa/vt82c686.c#L255

#define TYPE_VIA_SUPERIO "via-superio"
OBJECT_DECLARE_SIMPLE_TYPE(ViaSuperIOState, VIA_SUPERIO)

struct ViaSuperIOState {
     ISASuperIODevice superio;
     uint8_t regs[0x100];
     const MemoryRegionOps *io_ops;
     MemoryRegion io;
     MemoryRegion *serial_io[SUPERIO_MAX_SERIAL_PORTS];
};

[...]

static void via_superio_realize(DeviceState *d, Error **errp)
{
     ViaSuperIOState *s = VIA_SUPERIO(d);
     ISASuperIOClass *ic = ISA_SUPERIO_GET_CLASS(s);
     Error *local_err = NULL;
     int i;

     assert(s->io_ops);
     ic->parent_realize(d, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
     }
     /* Grab io regions of serial devices so we can control them */
     for (i = 0; i < ic->serial.count; i++) {
         ISADevice *sd = s->superio.serial[i];
         MemoryRegion *io = isa_address_space_io(sd);
         MemoryRegion *mr = find_subregion(sd, io, sd->ioport_id);
         if (!mr) {
             error_setg(errp, "Could not get io region for serial %d", i);
             return;
         }
         s->serial_io[i] = mr;
     }

     memory_region_init_io(&s->io, OBJECT(d), s->io_ops, s, "via-superio", 2);
     memory_region_set_enabled(&s->io, false);
     /* The floppy also uses 0x3f0 and 0x3f1 but this seems to work anyway */
     memory_region_add_subregion(isa_address_space_io(ISA_DEVICE(s)), 0x3f0,
                                 &s->io);
}

In realize we grab pointers to the MemoryRegions of the isa-serial devices 
created by the ISA superio class. This is ISA superio:

https://github.com/patchew-project/qemu/blob/ca5d88d2fee0016f939e91ae8b32c18e682064fa/include/hw/isa/superio.h#L23

#define SUPERIO_MAX_SERIAL_PORTS 4

struct ISASuperIODevice {
     /*< private >*/
     ISADevice parent_obj;
     /*< public >*/

     ISADevice *parallel[MAX_PARALLEL_PORTS];
     ISADevice *serial[SUPERIO_MAX_SERIAL_PORTS];
     ISADevice *floppy;
     ISADevice *kbc;
     ISADevice *ide;
};

The serial members we access are even public so this should be OK (other 
models to that too) but we need to get their MemoryRegion as we need to 
configure that based on VIA superio registers. ISADevice is defined in:

https://github.com/patchew-project/qemu/blob/patchew/cover.1615345138.git.balaton%40eik.bme.hu/include/hw/isa/isa.h

but it does not store a reference to its memory regions:

struct ISADevice {
     /*< private >*/
     DeviceState parent_obj;
     /*< public >*/

     int8_t isairq[2];      /* -1 = unassigned */
     int nirqs;
     int ioport_id;
};

only an ioport_id which is the address of its first io region so we have 
to get the actual MemoryRegion based on that. This works for isa-serial 
that has a single mem region but would not for parallel or FDC that have 
multiple regions. Those are created with isa_register_portio_list() I 
think but we don't care about parallel and FDC only about serial.

This may be possible to clean up but that would need changes to ISA 
emulation that I don't want to change so the patch tries to achieve what's 
needed without changing how ISA devices are emulated currently. I think 
the solution proposed in patch 1 is relatively clean and by not changing 
anything than vt82c686 it avoids any possible breakage to other machines 
using ISA devices so unless there's a reason not to accept it this should 
solve the problem and allow pegasos2 firmware to boot.

Regards,
BALATON Zoltan