Re: [PATCH 02/10] memory-device: Introduce memory_devices_init()

[David Hildenbrand]

On 30.05.23 15:04, David Hildenbrand wrote:
> On 30.05.23 14:29, Philippe Mathieu-Daudé wrote:
> > Hi David,
> >
> > On 30/5/23 13:38, David Hildenbrand wrote:
> > > Let's intrduce a new helper that we will use to replace existing memory
> > > device setup code during machine initialization. We'll enforce that the
> > > size has to be > 0.
> > >
> > > Once all machines were converted, we'll only allocate ms->device_memory
> > > if the size > 0.
> > >
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> > > ---
> > >     hw/mem/memory-device.c         | 14 ++++++++++++++
> > >     include/hw/mem/memory-device.h |  2 ++
> > >     2 files changed, 16 insertions(+)
> >
> >
> > > diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> > > index 48d2611fc5..6e8a10e2f5 100644
> > > --- a/include/hw/mem/memory-device.h
> > > +++ b/include/hw/mem/memory-device.h
> > > @@ -16,6 +16,7 @@
> > >     #include "hw/qdev-core.h"
> > >     #include "qapi/qapi-types-machine.h"
> > >     #include "qom/object.h"
> > > +#include "exec/hwaddr.h"
> > >     
> > >     #define TYPE_MEMORY_DEVICE "memory-device"
> > >     
> > > @@ -113,5 +114,6 @@ void memory_device_plug(MemoryDeviceState *md, MachineState *ms);
> > >     void memory_device_unplug(MemoryDeviceState *md, MachineState *ms);
> > >     uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
> > >                                            Error **errp);
> > > +void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size);
> >
> >
> > While hw/mem/memory-device.c contains the implementation, all callers
> > are expected to be around Machine object, right? Thus maybe this _init()
> > could be declared in "hw/boards.h", already included by machines
> > (eventually renaming as machine_init_memory_devices() ). Then machines
> > implementation don't have to all include "hw/mem/memory-device.h".
>
> Some (arm, i386) want to call the hotplug handle functions either way,
> so they'll still have to include that header.
>
> But sure, we can rename to machine_init_memory_devices() and declare it
> include/hw/boards.h!

FWIW, I went with "machine_memory_devices_init()", to mach the style of 
"machine_run_board_init()".

--
Thanks,

David / dhildenb