Am 5. März 2026 10:03:12 UTC schrieb Peter Maydell <[email protected]>:
>On Tue, 3 Mar 2026 at 22:22, Bernhard Beschow <[email protected]> wrote:
>>
>> Rather than requiring users of TYPE_SERIAL to initialize the MMIO region
>> themselves, make it generic enough to be configured via properties. This
>> makes TYPE_SERIAL more self-contained and prepares it for being turned
>> into a SysBusDevice.
>>
>> Signed-off-by: Bernhard Beschow <[email protected]>
>> ---
>> include/hw/char/serial-mm.h | 3 --
>> include/hw/char/serial.h | 4 +-
>> hw/char/diva-gsp.c | 5 ---
>> hw/char/serial-isa.c | 1 -
>> hw/char/serial-mm.c | 51 -------------------------
>> hw/char/serial-pci-multi.c | 5 ---
>> hw/char/serial-pci.c | 1 -
>> hw/char/serial.c | 76 ++++++++++++++++++++++++++++++-------
>> 8 files changed, 66 insertions(+), 80 deletions(-)
>
Hi Peter,
>
>
>This does leave TYPE_SERIAL_MM a bit of a do-nothing
>extra wrapper, as Balaton says. However I don't think
>it's being used by any versioned machine types,
Most prominently, the riscv and loongson virt machines use it, but aren't
versioned yet AFAICS.
> so we
>could maybe look after this at the possibility of dropping
>the wrapper (by having serial_mm_init() create a TYPE_SERIAL
>directly, and dropping TYPE_SERIAL_MM entirely). I don't
>think we strictly need to do that, though -- being able
>to clean up the reset handling here is worthwhile in
>itself.
>
>I do have one question:
>
>> + memory_region_init_io(&s->io, OBJECT(s),
>> + s->regshift ? &serial_mm_ops[s->endianness]
>> + : &serial_io_ops,
>> + s, "serial", 8 << s->regshift);
>
>This means that users of serial_mm_init() which pass 0 for
>regshift now get serial_io_ops rather than serial_mm_ops.
This is correct.
>That affects a handful of machines. I'm not sure if
>this makes a guest-visible difference (very possibly not),
>but for a "refactoring" type patch like this I'd rather
>err on the safe side and keep the same MemoryRegionOps
>if we can. This could be awkward, though, because we
>definitely have users of TYPE_SERIAL_MM which rely on
>the default regshift property value being 0. Maybe we
>can have the handful of places that want the serial_io_ops
>do an explicit setting of some property to request that?
>
>The other approach is to analyse the code and satisfy
>ourselves that serial_io_ops and serial_mm_ops with
>regshift == 0 really do have exactly identical behaviour,
>and note that in the commit message.
I've already sent v3 to fix the lifetime issue you pointed out and attempted to
take the last approach from above. But then realized that a justification in
the commit message isn't as straight forward as anticipated. I've left some
thoughts there...
Best regards,
Bernhard
>
>thanks
>-- PMM
