Re: [PATCH 2/3] softmmu/ioport.c: QOMify MemoryRegionPortioList

[Mark Cave-Ayland]

On 20/04/2023 09:41, Philippe Mathieu-Daudé wrote:

> On 19/4/23 17:16, Mark Cave-Ayland wrote:
> > The aim of QOMification is so that the lifetime of the MemoryRegionPortioList
> > structure can be managed using QOM's in-built refcounting instead of having to
> > handle this manually.
> >
> > Due to the use of an opaque pointer it isn't possible to model the new
> > TYPE_MEMORY_REGION_PORTIO_LIST directly using QOM properties, however since
> > use of the new object is restricted to the portio API we can simply set the
> > opaque pointer (and the heap-allocated port list) internally.
>
> In all uses this opaque pointer is a Object*. Almost all cases are
> a DeviceState* and one is a BusState* (IDEBus).
>
> Could this opaque become 'Object *owner' (simplifying the next patch)?

I'm not sure that it does, since the opaque is part of the portio API and not related 
to this series which is focused on MemoryRegionPortioList i.e. the glue interface 
between the portio and memory APIs. Otherwise you end up changing the portio API and 
the associated callbacks which is orthogonal to the bug this series is trying to fix.

> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   softmmu/ioport.c | 25 ++++++++++++++++++++++---
> >   1 file changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/softmmu/ioport.c b/softmmu/ioport.c
> > index d0d5b0bcaa..238625a36f 100644
> > --- a/softmmu/ioport.c
> > +++ b/softmmu/ioport.c
> > @@ -32,11 +32,16 @@
> >   #include "exec/address-spaces.h"
> >   #include "trace.h"
> > -typedef struct MemoryRegionPortioList {
> > +struct MemoryRegionPortioList {
> > +    Object obj;
> > +
> >       MemoryRegion mr;
> >       void *portio_opaque;
> >       MemoryRegionPortio *ports;
> > -} MemoryRegionPortioList;
> > +};


ATB,

Mark.