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

[David Hildenbrand <david@redhat.com>]

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